How to fill strings in a 2D char array of C?

Question

Trying to implement strtok.

#include <stdio.h>

char* my_strtok (char* arg_string, const char* arg_delimeter)
{
    char** after_tokenization      = (char**) malloc (sizeof(char) * 1000);
    char*  hold_chars_of_one_group = (char*) malloc (sizeof(char) * strlen(arg_string));

    int j = 0;
    int i = 0;
    int k = 0;

    while (arg_string[i])
    {
        if (arg_string[i] != *arg_delimeter)
        {
            hold_chars_of_one_group[k] = arg_string[i];
            k++;
        }
        else
        {
            after_tokenization[j][0] = hold_chars_of_one_group;

            j++;
            k = 0;
        }
        i++;
    }

    return after_tokenization;
}


int main(void)
{
    char*p = my_strtok ("qwerty,asdf,shizuka,sharma", ",");
    return 0;
}

By putting printfs I can see that seg fault is in this line:

after_tokenization[j][0] = hold_chars_of_one_group;

Before crashing j's value is shown to be 2. Sufficient memory has been allocated to both arrays, then what is the way to push values in a 2D char array of C?

Why am I getting seg fault over there? What's the way out?

Answer 1

after_tokenization[j][0] = hold_chars_of_one_group;

Even if you allocated enough memory for after_tokenization. The pointer at after_tokenization[j] isn't initialized. It contains an unspecifed address. So when you dereference it by applying the subscript operator [0], it's undefiend behavior.

And that is most probably the cause of your crash.

Answer 2

You need an array of pointers to hold the strings, so it should be:

char** after_tokenization      = (char**) malloc (sizeof(char*) * 1000);

and after_tokenization[j][0] is meaningless because after_tokenization[j] is just a pointer, you haven't allocate memory for it. Here is the modified version according to your code.

char** my_strtok (char* arg_string, const char* arg_delimeter)
{
    char** after_tokenization      = (char**) malloc (sizeof(char*) * 1000);
    char*  hold_chars_of_one_group = (char*) calloc(strlen(arg_string) + 1, sizeof(char)); // use calloc to fill the memory with bytes of value zero.

    int j = 0;
    int i = 0;
    int k = 0;

    while (arg_string[i])
    {
        if (arg_string[i] != *arg_delimeter)
        {
            hold_chars_of_one_group[k] = arg_string[i];
            k++;
        }
        else
        {
            hold_chars_of_one_group[k] = 0;
            after_tokenization[j] = hold_chars_of_one_group;
            hold_chars_of_one_group += k+1;

            j++;
            k = 0;
        }
        i++;
    }

    // last one
    if (hold_chars_of_one_group[0] != 0) {
        hold_chars_of_one_group[k] = 0;
        after_tokenization[j] = hold_chars_of_one_group;
    }

    /*for (i = 0; i < 10; i++) {
        printf("%s\n", after_tokenization[i]);
    } */


    return after_tokenization;
}

Answer 3

I believe their are some problems with you code:

• You are typecasting return of malloc(). Their is no need for that in C. Please read this.

• Allocation for after_tokenization is wrong. You need to allocate space for char * pointers, not char characters. it needs to be allocated like this:

  char** after_tokenization = malloc (sizeof(char*) * 1000);
  

• Return of malloc() needs to be checked, as it can return NULL.

• This line:

  after_tokenization[j][0] = hold_chars_of_one_group;
  

  is dangerous, as you are not really copying hold_chars_of_one_group into your array. You need to malloc() some space for this, then strcpy() it into the array. Their is multiple methods for this.

  Your current code just overwrites the address of previous pointers added. Their also no need for [j][0], as you only need to copy into a pointer location [j].

• strtok() can take multiple delimiters, but your code only handles 1. This isn't really a problem, just something to consider.

• my_strtok() returns char *, but you are returning char ** in this function. You need to change this to char **my_strtok().

• You also need to free() any allocated memory at the end.

These points will help improve your code, and make it functional.

Here is some example code:

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

#define MAXSTR 1000

char **mystrtok(char *arg_string, const char *arg_delimeter);

int main(void) {
    char **result = NULL;

    result = mystrtok("qwerty,asdf,shizuka,sharma", ",");

    /* printing and freeing strings */
    for (size_t i = 0; result[i] != NULL; i++) {
        printf("%s\n", result[i]);
        free(result[i]);
        result[i] = NULL;
    }

    free(result);
    result = NULL;

    return 0;
}

char **mystrtok(char *arg_string, const char *arg_delimeter) {
    char **after_tokenization = NULL;
    char *group_char = NULL;
    size_t arrsize = MAXSTR, slen, count = 0, numstr = 0, delim_flag;

    /* allocation of array, with error checking */
    after_tokenization = malloc(arrsize * sizeof * after_tokenization);
    if (!after_tokenization) {
        printf("Cannot allocate %zu spaces for pointers\n", arrsize);
        exit(EXIT_FAILURE);
    }

    slen = strlen(arg_string);

    /* allocation of buffer, with error checking */
    group_char = malloc(slen+1);
    if (!group_char) {
        printf("Cannot allocate %zu bytes for string\n", slen+1);
        exit(EXIT_FAILURE);
    }

    for (size_t ch = 0; arg_string[ch]; ch++) {
        delim_flag = 0;

        /* loop to handle multiple delimeters */
        for (size_t del = 0; arg_delimeter[del]; del++) {
            if (arg_string[ch] == arg_delimeter[del]) {
                delim_flag = 1;
            }
        }

        /* no delim found, add to buffer */
        if (!delim_flag) {
            group_char[count++] = arg_string[ch];
            group_char[count] = '\0';

        /* only add if delim found and buffer is not NULL */
        } else if (delim_flag && *group_char) {

            /* make space in array */
            after_tokenization[numstr] = malloc(slen+1);
            if (!after_tokenization[numstr]) {
                printf("Cannot allocate %zu bytes for string\n", slen+1);
                exit(EXIT_FAILURE);
            }

            /* copy buffer into array */
            strcpy(after_tokenization[numstr], group_char);

            numstr++;
            count = 0;

            /* clear buffer */
            memset(group_char, '\0', slen+1);
        }
    }

    /* for last string found */
    if (*group_char) {
        after_tokenization[numstr] = malloc(slen+1);
        if (!after_tokenization[numstr]) {
            printf("Cannot allocate %zu bytes for string\n", slen+1);
            exit(EXIT_FAILURE);
        }

        strcpy(after_tokenization[numstr], group_char);
        numstr++;
    }

    /* free buffer, not longer needed */
    free(group_char);

    /* add sentinel, just in case */
    after_tokenization[numstr] = NULL;

    /* return char** at the end */
    return after_tokenization;
}

Note: This is just some code I wrote, and it can be heavily improved. It just shows the idea.

Answer 4

fix like this

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

char** my_strtok (char* arg_string, const char* arg_delimeter){
    size_t len = strlen(arg_string);
    char **after_tokenization     = malloc(sizeof(char*) * ((len + 1)/2 +1));//max token + 1 (+1 for NULL)
    char *hold_chars_of_one_group = malloc(len + 1);

    int i, j, k;
    i = j = k = 0;

    while (*arg_string){
        if (*arg_string != *arg_delimeter){
            hold_chars_of_one_group[k++] = *arg_string;
        } else  {
            hold_chars_of_one_group[k++] = 0;
            after_tokenization[j++] = &hold_chars_of_one_group[i];
            i = k;
        }
        ++arg_string;
    }
    hold_chars_of_one_group[k] = 0;
    after_tokenization[j++] = &hold_chars_of_one_group[i];
    after_tokenization[j] = NULL;//NULL is terminator

    return after_tokenization;
}

int main(void){
    char **p = my_strtok ("anisha,kaul,shizuka,sharma", ",");
    for(char **temp = p; *temp; ++temp){
        printf ("-%s-\n", *temp);
    }
    free(*p);
    free(p);
    return 0;
}