Split string with delimiter in C - wrong count, does not split properly

Question

I want to split string (line) of words with a given delimiter. I wrote this code below:

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

unsigned int countWords(char *stringLine)
{
    unsigned int count = 0;
    char* tmp = stringLine;
    char* last = 0;
    const char delim = '/';

    while (*tmp)
    {
        if (delim == *tmp)
        {
            count++;
            last = tmp;
        }
        tmp++;
    }
    return count-1;
}

char **getWordsFromString(char *stringLine)
{
    char** sizeNames = 0;
    unsigned int count = 0;
    const char *delim = "/";

    count = countWords(stringLine);

    sizeNames = malloc(sizeof(char*) * count);
    if(sizeNames == NULL)
    {
        return NULL;
    }

    if (sizeNames)
    {
        size_t idx  = 0;
        char* token = strtok(stringLine, delim);
        while (token)
        {
            if(idx < count)
            {
                exit(-1);
            }
            *(sizeNames + idx++) = strdup(token);
            token = strtok(0, delim);
        }
        if(idx == count - 1)
        {
            exit(-1);
        }
        *(sizeNames + idx) = 0;
    }

    return sizeNames;
}

void showWords(char *stringLine)
{
    unsigned int size = countWords(stringLine), i = 0;
    char** sizeNames = getWordsFromString(stringLine);
    for(i=0; i<size; i++)
    {
        printf("%s\n", sizeNames[i]);
    }
}

int main()
{
    char words[] = "hello/world/!/its/me";

    int c = countWords(words);
    printf("c = %d\n", c);
    showWords(words);
    return 0;
}

The problem is that my code does not work right. It shows a proper number of words in a string but does not split them, only gives me 255 return code. How can I improve it?

Answer 1

Here:

size_t idx  = 0;
char* token = strtok(stringLine, delim);
while (token)
{
    if(idx < count)
    {
        exit(-1);
    }

given that you set idx to 0, as long as count is greater than zero you'll always exit() here. Likely you want a different if condition. Exit codes are probably unsigned on your system, so that -1 is just getting translated to 255.

Answer 2

I personally prefer using for-loop notation with strtok... and I would write the core loop like this:

if (sizeNames)
{                                                                                                                       
    size_t idx  = 0;
    char *token;
    for( token = strtok(stringLine, delim);
         token && idx<count;
         token = strtok(NULL, delim), idx++)
    {
        *(sizeNames + idx) = strdup(token);
    }
    *(sizeNames + idx) = NULL;
}

But, since you are NULL-terminating after the last word, be sure to use count+1 in the following, or else the above line *(sizeNames + idx) = NULL; could segfault:

sizeNames = malloc(sizeof(char*) * (count+1));

PS... your countWords routine returns the wrong answer, too. Probably want to replace:

return count-1;

With:

if( (last+1)==tmp )
   return count;                                                                                                        

return count+1;

Also, the memory you allocate with strdup() is never freed anywhere, so you will get leak errors if you run this through valgrind.

Note, you don't really have to use strdup() since strtok() leaves NULs in the places where it found delimiters in the source string, so you could have written:

*(sizeNames + idx) = token;

in the heart of the for-loop. Because of this destructive behavior, strtok() is sometimes only run on a copy of a buffer to be analyzed.