Segmentation Fault using Strtok() for 2d array in C

Question

I keep getting Segmentation Fault while trying to split a string such as "1,2,3;4,5,6;7,8,9" into a 2d Array why does it keep happening with this code could someone please explain this for me?

int main(void){
    char string[100];
    char *token;
    char *end;
    int num;
    int row_counter;
    int column_counter;
    int counter_position;
    char *rows[100];
    int square_ints[40][40];

    row_counter = 0;
    column_counter = 0;
    string[] = "1,2,3;4,5,6;7,8,9";
    token = strtok(string, ";");
    rows[row_counter] = token;
    row_counter++;
    while (token != NULL){
        token = strtok(NULL, ";");
        rows[row_counter] = token;
        row_counter++;
    }

    counter_position = row_counter;

    for (row_counter = 0; row_counter < counter_position; row_counter++) {
        token = strtok(rows[row_counter], ",");
        num = strtol(token, &end, 10);
        square_ints[row_counter][column_counter] = num;
        printf("%d
", square_ints[row_counter][column_counter]);
        column_counter++;
        while (token != NULL) {
            token = strtok(NULL, ",");
            num = strtol(token, &end, 10);
            square_ints[row_counter][column_counter] = num;
            printf("%d
", square_ints[row_counter][column_counter]);
            column_counter++;
        }
    }
}

It should be printing: 1 2 3 4 5 6 7 8 9 instead I get 1 2 3 Segmentation Fault

Answer 1

To expand on what MikeCAT mentioned:

As the start of your for loop, you do:

token = strtok(rows[row_counter], ",");
num = strtol(token, &end, 10);

Later you do:

token = strtok(NULL, ",");
num = strtol(token, &end, 10);

The problem is that in both cases, strtok will return NULL, so the value of token is NULL. When you pass token to strtol, it will try to use/dereference this and it will produce a segfault.

So, to fix, after each strtok call, you need to add:

if (token == NULL)
    break;

---

Here's the corrected code. I've added some debug printf, so you can see the issue.

I've used cpp conditionals to demarcate old vs new code:

#if 0
// old code
#else
// new code
#endif

#if 1
// new code
#endif

Anyway, here it is:

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

#ifdef DEBUG
#define dbgprt(_fmt...) 
    do { 
        printf(_fmt); 
    } while (0)
#else
#define dbgprt(_fmt...) 
    do { 
    } while (0)
#endif

int
main(void)
{
#if 0
    char string[100];
#else
    char string[] = "1,2,3;4,5,6;7,8,9";
#endif
    char *token;
    char *end;
    int num;
    int row_counter;
    int column_counter;
    int counter_position;
    char *rows[100];
    int square_ints[40][40];

    row_counter = 0;
    column_counter = 0;
#if 0
    string[] = "1,2,3;4,5,6;7,8,9";
#endif
    token = strtok(string, ";");
    rows[row_counter] = token;
    row_counter++;
    while (token != NULL) {
        token = strtok(NULL, ";");
        rows[row_counter] = token;
        row_counter++;
    }

    counter_position = row_counter;

    for (row_counter = 0; row_counter < counter_position; row_counter++) {
        dbgprt("DEBUG: rows[%d]='%s'
",row_counter,rows[row_counter]);
        token = strtok(rows[row_counter], ",");
        dbgprt("DEBUG: token='%s'
",token);
#if 1
        if (token == NULL)
            break;
#endif
        num = strtol(token, &end, 10);
        square_ints[row_counter][column_counter] = num;
        printf("%d
", square_ints[row_counter][column_counter]);
        column_counter++;
        while (token != NULL) {
            token = strtok(NULL, ",");
            dbgprt("DEBUG2: token='%s'
",token);
#if 1
            if (token == NULL)
                break;
#endif
            num = strtol(token, &end, 10);
            square_ints[row_counter][column_counter] = num;
            printf("%d
", square_ints[row_counter][column_counter]);
            column_counter++;
        }
    }

    return 0;
}

---

Note that while the above code works, it is replicating code before a given loop and inside the loop. If we add an extra pointer variable (e.g. bp), we can simplify the code a bit:

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

int
main(void)
{
    char string[] = "1,2,3;4,5,6;7,8,9";
    char *token;
    char *end;
    int num;
    int row_counter;
    int column_counter;
    int counter_position;
    char *bp;
    char *rows[100];
    int square_ints[40][40];

    row_counter = 0;
    column_counter = 0;

    bp = string;
    while (1) {
        token = strtok(bp,";");
        bp = NULL;
        if (token == NULL)
            break;
        rows[row_counter] = token;
        row_counter++;
    }

    counter_position = row_counter;

    for (row_counter = 0; row_counter < counter_position; row_counter++) {
        bp = rows[row_counter];
        if (bp == NULL)
            break;

        while (1) {
            token = strtok(bp, ",");
            bp = NULL;
            if (token == NULL)
                break;

            num = strtol(token, &end, 10);
            square_ints[row_counter][column_counter] = num;
            printf("%d
", square_ints[row_counter][column_counter]);
            column_counter++;
        }
    }

    return 0;
}

---

There is another similar function strsep that is considered by some to be "strtok done right". Here's a version that uses that function:

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

int
main(void)
{
    char string[] = "1,2,3;4,5,6;7,8,9";
    char *token;
    char *end;
    int num;
    int row_counter;
    int column_counter;
    int counter_position;
    char *bp;
    char *rows[100];
    int square_ints[40][40];

    row_counter = 0;
    column_counter = 0;

    bp = string;
    while (1) {
        token = strsep(&bp,";");
        if (token == NULL)
            break;
        rows[row_counter] = token;
        row_counter++;
    }

    counter_position = row_counter;

    for (row_counter = 0; row_counter < counter_position; row_counter++) {
        bp = rows[row_counter];
        if (bp == NULL)
            break;

        while (1) {
            token = strsep(&bp,",");
            if (token == NULL)
                break;

            num = strtol(token, &end, 10);
            square_ints[row_counter][column_counter] = num;
            printf("%d
", square_ints[row_counter][column_counter]);
            column_counter++;
        }
    }

    return 0;
}