Thread: Code almost works, but not exactly, need a bit of help

Just trying to write some code for personal practice, not homework or anything of that sort.


Some weird happens: removing

Code:

char *wordArray1[10] = {0};

line in the full code below causes the program to work fine until it prints the first value of 3 to indicate the word olly has shown up 3 times then the program stops and exits the console. Leaving the above code alone results in the program doing what I expected, which is to count number of times each word in the string shows up in said string. Why is that? wordArray1 is a useless variable that I wanted to use but decided it was not needed.

I am using CodeLite/GCC compiler, not sure if that matters any to my question above and also any tips to improve full code would be appreciated. Thank you

Code:

// Word Count: Given a phrase, count the occurrences of each word in that phrase.
// I am assuming case insensitivity.
#include <stdio.h>
#include <string.h>
#include <ctype.h>

int main()
{
    char str[] = "olly olly olly in come free";
    char *token = strtok(str, " "); // get first token
    char *wordArray[10] = {0};
    char *wordArray1[10] = {0};
    int i = 0;
    int wordCount = 0;


    // convert phrase to lowercase
    for(int j=0; j < strlen(str); j++)
    {
        str[j] = tolower(str[j]);
    }


    // walk through other tokens
    while( token != NULL )
    {
        wordCount++;
        wordArray[i++] = token;
        token = strtok(NULL, " ");
    }


    // print the words as is
    for (i = 0; i <  wordCount; i++)
    {
        printf("%s ", wordArray[i]);
    }


    printf("\n\n");


    int countArray[] = {0}; // store the entire list of word frequency counts
    for (i = 0; i <  sizeof(wordArray) / sizeof(wordArray[0]); i++)
    {


        if(wordArray[i] != NULL) // check if current word has been processed or not (processed == NULL'd)
        {
            countArray[i] = 1; // New word detected


            for(int j = i + 1; j < sizeof(wordArray) / sizeof(wordArray[0]); j++) // loop through the rest of the words
            {
                if( (wordArray[j] != NULL) && (strcmp(wordArray[j], wordArray[i]) == 0) )
                {
                    // found a duplicate word,
                    // count for that word increments by 1.
                    countArray[i] += 1;


                    // word at index j already processed, so set its value to NULL
                    wordArray[j] = NULL;
                }
            }
        }
        else
        {
            // NULL indicates duplicate, set count to 0 for consistency
            countArray[i] = 0;
        }
    }

    for (int k = 0; k < wordCount; k++)
    {
        if(wordArray[k] != NULL)
        {
            printf("%s: ", wordArray[k]);
        }


        if(countArray[k] != 0)
        {
            printf("%d\n", countArray[k]);
        }
    }
    printf("\n");


    return 0;
}

You're right: what you're looking at is the effect of undefined behaviour.

First thing that I noticed is not related to your problem, but is a potential problem anyway:

Code:

char *token = strtok(str, " "); // get first token

You probably only want to call strtok after the tolower loop. Otherwise, the words after the first word will not be changed to all lowercase.

This is likely the crucial mistake:

Code:

int countArray[] = {0};

Given that you expect to have at most 10 words, this should be:

Code:

int countArray[10] = {0};

Otherwise, you only have space for a single count, resulting in buffer overflow and hence the undefined behaviour.

Speaking of the 10 word limit, I note that your strtok loop should also check that this limit is not exceeded, otherwise you will risk buffer overflow.

EDIT:
Oh, this loop should loop based on wordCount rather than always looping over the entire wordArray:

Code:

for (i = 0; i <  sizeof(wordArray) / sizeof(wordArray[0]); i++)