CODEWITHSUNDEEP
cstring
wing
wing

Reputation: 67

What is wrong with the function strtok()?

So, how it works? I've the following block of code;

for (i = 0; i < N; ++i){
    array[i] = malloc(M * sizeof(char));
    fgets(buffer, M-1, fp);
    array[i] = strtok(buffer, "\n");
}

If I execute printf("%s", array[i]); inside the for the strtok() works as I expected, but if I do 

for (i = 0; i < N; ++i)
    printf("%s", array[i]);

outside the previous for, I obtain a loop of N array[0].

Upvotes: 2

Views: 1278

Answers (2)

Sourav Ghosh
Sourav Ghosh

Reputation: 134286

What is wrong with the function strtok()?

Nothing, function works as expected. The way it is used here, creates the problem.

Note: You have already received the answer from Mr. dasblinkenlight, however, just to elaborate another missing point, adding this answer.

The problems in your code are

  • Point 1. As already mentioned, strtok() does not return a value that is persistent among successive calls. You've to copy the value of the returned token in each call, otherwise, you'll not get the expected value. For that, either you have to (after a sanity check of NULL token)

    • 1.1. Take a pointer, allocate memory (using malloc() or family) and copy the token content using strcpy()

    • 1.2. use (non-standard) strdup() to directly copy the token to array[i]. No need of allocating memory separately.

    FWIW, for both the cases, one you're done with the array[i], you need to free() it.

  • Point 2: By allocating memory to array[i] using malloc() 

    array[i] = malloc(M * sizeof(char));

    and then assigning another pointer to it

    array[i] =  //some pointer...

    you're overwriting the previously allocated pointer, thereby facing memory leak. Avoid this design and follow either of the above ways to get your code to work properly.

That said,

  1. (just an advice) sizeof(char) is guranteed to produce 1 according to C standard. Multiplication by sizeof(char) is likely to be avoided.
  2. Always check the return value of fgets() to ensure a successful read.

Upvotes: 2

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726489

The problem with strtok is that the token it returns you becomes invalid as soon as you make the next call of strtok. You could either copy it into a separate string, or use it right away, and discard, but you must use it before calling strtok again.

for (i = 0; i < N; ++i){
    array[i] = malloc(M * sizeof(char));
    fgets(buffer, M-1, fp);
    char *tok = strtok(buffer, "\n");
    array[i] = malloc(strlen(tok)+1);
    strcpy(array[i], tok);
}

Of course now you need to call free on all elements of your array[] after you are done with them.

Note that strtok is not re-entrant, which severely limits its use. A better choice is strtok_r, which is a re-entrant version of the same function.

Upvotes: 1

Related Questions