Thread: Help with parsing a sentence.

I am writing a function to parse a sentence and print each separate word on a new line.
For some reason, I keep getting a Seg fault. Can anyone please tell me what I am doing wrong?
It is separating the words and printing them each on a new line, but then it Seg faults after the last word is printed.
Any advice would be much appreciated.

Code:

//prototype
void PrintList(char *ptrA[], const int size);

//call in main looks like this
ParseSentence(Buffer3);


//function
void ParseSentence(char *ptrA)
{
	char *words[80];
	const char delimiters[4] = {' ','.',',',';'};
	
	int size = strlen(ptrA);

	words[0] = strtok(ptrA, delimiters);
	printf("%s\n", words[0]);
	
	for(int i = 1; i > 0 && i < size - 1; i++)
	{
		words[i] = strtok(NULL, delimiters);
		printf("%s\n", words[i]);
	}

}

Originally Posted by vart

second parameter of strtok is not array of char - it should be a nul-terminated string

also - you should check the return value of strtok before using it

OK, I changed the code to the following.
Still getting the same result.

Code:

//prototype  *was wrong in initial post*
void ParseSentence(char *ptrA);

//call in main
ParseSentence(Buffer3);

//function
void ParseSentence(char *ptrA)
{
	char *words[80];
	
	int size = strlen(ptrA);

	words[0] = strtok(ptrA, " .,;");
	printf("%s\n", words[0]);
	
	for(int i = 1; i > 0 && i < size - 1; i++)
	{
		words[i] = strtok(NULL, " .,;");
		printf("%s\n", words[i]);
	}

}

If you have a 40-character string, the odds of you have 40 space-separated words in that string is zero. So your loop can't go by the number of characters, it needs to go until you run out of words.

Should I be copying the char array being passed to the function into a temp array before using strtok?

If so, how do I do that? I'm kinda new to C.

Originally Posted by w2look

Should I be copying the char array being passed to the function into a temp array before using strtok?

If so, how do I do that? I'm kinda new to C.

strtok will change the array passed in. If that's bad, then you need to make a copy. It's called strcpy.

Originally Posted by tabstop

If you have a 40-character string, the odds of you have 40 space-separated words in that string is zero. So your loop can't go by the number of characters, it needs to go until you run out of words.

Do I use an if statement for that?

like

if (character == NULL)
break;

Tried changing it to this, but same result.

Code:

void ParseSentence(char *ptrA)
{
	char temp[80];
	char *words[80];
	strcpy(temp, ptrA);
	
	int size = strlen(temp);

	words[0] = strtok(temp, " .,;");
	
	printf("%s\n", words[0]);
	
	for(int i = 1; i < size - 1; i++)
	{
		words[i] = strtok(NULL, " .,;");
		printf("%s\n", words[i]);
	}

}

I thought that there may be a possibility of another part of my program causing the problem
so I put everything regarding this function into it's own program to try and isolate the problem.
Here is the code I'm using:

Code:

#include <stdio.h>
#include <string.h>

//prototype
void ParseSentence(char *ptrA);

int main()						/* function main begins program execution */
{
	/* initialize and declare variables and containers */
	char Buffer3[80];
	
	/* prompt the user for input */
	printf("Please type a string of characters to be stored in Buffer3\n");
	/* scanf("%s", Buffer3);   Since this method does not work we can use gets instead */
	
	gets(Buffer3);
	
	printf("\n\n");
	ParseSentence(Buffer3);
	printf("\n\n");

    return 0;
}

//Break a sentence up into words and print them on separate lines
//Input: a NULL terminated string
//Output:a printed list of the words 
void ParseSentence(char *ptrA)
{
	char temp[80];

	strcpy(temp, ptrA);
	
	int size = strlen(temp);
	char *words[size];

	words[0] = strtok(temp, " .,;");
	
	printf("%s\n", words[0]);
	
	for(int i = 1; i < size - 1; i++)
	{
		words[i] = strtok(NULL, " .,;");
		printf("%s\n", words[i]);
		
		if(words[i] == NULL)
		{
			break;
		}
	}

}

Sample Input looks like:

This, my friends; is a string. of chars.

The output I get is:

This
my
friends
is
a
string
of
chars
Segmentation fault (core dumped)

Any suggestions as to how I can fix this?

Since your string has a length of 40, you are trying to print 40 words. Since you don't have 40 words, the computer complains. You must not use the length of the string to control how many words you print. You need to print until you run out of words. Look up strtok to see what happens when you run out of words.

Originally Posted by tabstop

Since your string has a length of 40, you are trying to print 40 words. Since you don't have 40 words, the computer complains. You must not use the length of the string to control how many words you print. You need to print until you run out of words. Look up strtok to see what happens when you run out of words.

I'm not quite sure where you got the # 40 from. The default size of the buffer for user input is 80 chars. But what you suggest makes sense. I guess I should be using a while loop instead of a for loop. I'll give it a try and see what happens.

Originally Posted by w2look

I'm not quite sure where you got the # 40 from. The default size of the buffer for user input is 80 chars. But what you suggest makes sense. I guess I should be using a while loop instead of a for loop. I'll give it a try and see what happens.

I got the 40 from scrolling too far and reading the previous code that used strlen. So yes, this code is trying to print 80 words, and you also don't have that many words.

looks like I spoke too soon.

no more seg fault, but not printing past the first word

hmm?

I'm guessing you have to fill the words array before doing the loop? Right now, you're checking whether words[1] exists, then if so, assign words[1]. Presumably it should go the other way around.