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]);
        }

}

---

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

Quote:

---

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.

Quote:

---

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.

Quote:

---

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.

Quote:

---

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.

Quote:

---

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.

I don't know what I was thinking, I didn't need a while loop at all.
If I was smarter and read your advice thoroughly, I would have saved myself some time.

i.e. "You need to print until you run out of words."

This simple change fixed it.

Code:

---

void ParseSentence(char *ptrA)
{
        char temp[80];

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

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

}

---

No more Seg fault.

Thanks for talking me through it tabstop. :)

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.

OK, this time I think I've got it.

Code:

---

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

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

---

Seems to work good. Tried it with multiple input, varying punctuation.

Thanks again tabstop.

Ehm, no!

You now have a loop inside another loop - and it checks a variable that hasn't been set yet (words[i] is not defined).

--
Mats

Quote:

---

Originally Posted by matsp

Ehm, no!

You now have a loop inside another loop - and it checks a variable that hasn't been set yet (words[i] is not defined).

--
Mats

---

I see what you're saying, I have since moved on to the next part of this problem and it is causing me problems. While I am getting the printed output I desired, the words are not being stored in the indexes of the array words[].

Any suggestions on a fix?

Quote:

---

Originally Posted by w2look

I see what you're saying, I have since moved on to the next part of this problem and it is causing me problems. While I am getting the printed output I desired, the words are not being stored in the indexes of the array words[].

Any suggestions on a fix?

---

Can you show what the code for the next part of the problem looks like?

Actually, I think I got it!

Changed the code in the for loop to:

Code:

---

        for(i = 1; i < size - 1; i++)
        {

                        words[i] = strtok(NULL, " .,;");
                       
                        if(words[i] == NULL)
                        {
                                break;
                        }
                        count++;
        }

---

Now, I believe it works correctly. Thanks for clarifying my mistake Mats!

Quote:

---

Originally Posted by itCbitC

Can you show what the code for the next part of the problem looks like?

---

Why not?

The next part of the problem involved taking the newly "parsed" array of words, sorting them in alphabetical order and then printing them.

Code:

---

//prototypes
void ParseSentence(char *ptrA);
void BubbleSort(char *ptrA[], const int size);
void PrintList(char *ptrA[], const int size);

//call in main
ParseSentence(Buffer3);

//Break a sentence up into words, sort them alphabetically and print them
//Input: a NULL terminated string
//Output:a printed list of the words in the string sorted alphabetically
void ParseSentence(char *ptrA)
{
        char temp[80];
        int i, j;
        int count = 1;
       
        strcpy(temp, ptrA);
       
        int size = strlen(temp);
        char *words[80];

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

                        words[i] = strtok(NULL, " .,;");
                       
                        if(words[i] == NULL)
                        {
                                break;
                        }
                        count++;
        }

        printf("Buffer3 in alphabetical order(Capital letters have priority):\n\n");
        BubbleSort(words, count);
        PrintList(words, count);
}

//Now that we have an array of words, we need to sort them alphabetically
//Input: an array or words and it's size
//Output: the words in the array sorted alphabetically
void BubbleSort(char *ptrA[], const int size)
{
  int pass;
  int j;
  char *tempPtr;

  for (pass=0; pass < size - 1; pass++)
  {
      for (j=0; j<size - 1; j++)
      {
        if (strcmp(ptrA[j], ptrA[j + 1]) > 0)
        {
            /* swap pointers */
            tempPtr = ptrA[j];
            ptrA[j] = ptrA[j+1];
            ptrA[j+1] = tempPtr;
        }
      }
  }
}

//Once the list of words is sorted, we need to print it
//Input: the array of words to be printed and it's size
//Output:
void PrintList(char *ptrA[], const int size)
{
  int i;
  for (i = 0; i < size; i++)
  {
      printf("%s\n", ptrA[i]);
  }
}

---