Tokenizing string using strtok

Question

In this assignment, I'm supposed to split a string using strtok. (The assignment consists of the comment and the function definition; I fill in the function body.)

It works as far as I've tested. But it seems a little ugly to me. Perhaps it's my Java background, but somehow this just doesn't feel right. Specific concerns:

• Is this easily readable to a more experienced C programmer?
• Can this be made more idiomatic somewhere?
• Is it feasible to split this into several functions? Where would you split it?
• Any corner cases where it might break? Does it leak memory?

/* Parses a string into tokens (command line parameters) separated by space.
 * Builds a dynamically allocated array of strings. Pointer to the array is
 * stored in variable pointed by argv.
 * 
 * Parameters:
 * argv: pointer to the variable that will store the string array
 * input: the string to be parsed (the original string can be modified, if needed)
 * 
 * Returns:
 * number of space-separated tokens in the string */
int parse_cmdline(char ***argv, char *input)
{
    char *token;
    int len, num;

    len = 4;
    *argv = malloc(len * sizeof(char *));
    if (*argv == NULL) {
        return 0;
    }
    for (num = 0;; num++, input = NULL) {
        token = strtok(input, " ");
        if (token == NULL) {
            break;
        }
        if (num >= len) {
            char **temp;

            len *= 2;
            temp = realloc(*argv, len * sizeof(char *));
            if (temp == NULL) {
                for (int i = 0; i < num; i++) {
                    free((*argv)[i]);
                }
                free(*argv);
                *argv = NULL;
                return 0;
            }
            *argv = temp;
        }
        (*argv)[num] = strdup(token);
        if ((*argv)[num] == NULL) {
            for (int i = 0; i < num; i++) {
                free((*argv)[i]);
            }
            free(*argv);
            *argv = NULL;
            return 0;
        }
    }
    return num;
}

Answer 1

The key point in tokenizing strings is that

The number of resulting token would not be known in prior

One good approach is the one you have followed,

• Allocate some memory
• Use it
• Allocate more if needed

I want to answer some of the questions that you have asked.

Is it feasible to split this into several functions? Where would you split it?

As you see, there is the repetition of error handling (freeing up the argv array). Well this can be separated into another function.

Is this easily readable to a more experienced C programmer?

Not sure of the level of expertise you mean, but I can read it and understand without any problems. I am a 2 year experienced C Professional.

Any corner cases where it might break? Does it leak memory?

There is no obvious memory leak I could see in the program. But you can use tools like valgrind to detect such leaks.

I am just posting few code, that I would use, if I am given the same program.

int free_argv_q(struct queue *q)
{
    int i;
    int q_len = q_length(q);

    for (i = 0; i < q_len; i++) {
        char *arg = dequeue(q);
        free(arg);
    }

    return 0;
}

int parse_cmdline(char ***argv, char *input)
{
    int i;
    int argc = 0;

    char *arg = NULL;
    char *token = NULL;

    struct queue q;

    token = strtok(input, " ");

    do {
        /* strdup may fail due to low memory */
        arg = strdup(token);

        if (arg == NULL) {
            free_argv_q(&q);
            return 0;
        }

        /* add to queue */
        enqueue(&q, arg);
        token = strtok(NULL, " ");
    } while (token);

    /* length of the queue is the number of splits */
    argc = q_length(&q);

    *argv = malloc(argc * sizeof(char *));

    if (*argv == NULL) {
        free_argv_q(&q);
        return 0;
    }

    /* now we have all memory allocated */
    for (i = 0; i < argc; i++)
        argv[i] = dequeue(&q);

    return argc;
}

The explanation is as follows.

• I use a queue to split the tokens and store them. Well this would grow dynamically.
• After parsing whole input, the argv array is allocated
• argc is the length of the queue
• Memory allocation failures are handled though
• The cleanup function is separated

Note: The implementation of queue is skipped for simplicity. Hope this program is self explanatory.