Firebird
Firebird

Reputation: 11

Segfault on accessing elements of dynamic 2d array

My goal is to create a function that converts a string into an array of "words" resulted from splitting an initial string by a delimiter. All words should be null-terminated. For example: strtoarr("**hello***world*", "*") should result in {"hello", "world"}. Here's my function.

char    **strtoarr(char const *s, char c)
{
    char    **arr;
    size_t  i;
    size_t  j;
    size_t  k;

    arr = malloc(sizeof(**arr) * (strlen(s) + 2));
    if (arr == 0)
        return (NULL);
    i = 0;
    k = 0;
    while (s[i] != '\0')
    {
        j = 0;
        while (s[i] != c)
        {
            arr[k][j] = s[i];
            if (s[i + 1] == c || s[i + 1] == '\0')
            {
                j++;
                arr[k][j] = '\0';
                k++;
            }
            i++;
            j++;
        }
        j = 0;
        while (s[i] == c)
            i++;
    }
    arr[k] = 0;
    return (arr);
}

It works only with empty strings and segfaults with everything else. I believe the problem is here.

arr[k][j] = s[i];

But I don't understand what the problem is. Thanks in advance

Upvotes: 0

Views: 74

Answers (2)

4386427
4386427

Reputation: 44274

There are a number of problems with your code but the most important is the dynamic allocation. Your code does not allocate memory for saving an array of strings (aka an array of array of char).

This line:

arr = malloc(sizeof(**arr) * (strlen(s) + 2));

allocates memory for saving a number chars (i.e. strlen(s) + 2 chars) but that is not what you want. Especially not when arr is a pointer to pointer to char.

A simple approach that you can use is to allocate an array of char pointers and then for each of these pointers allocate an array of char.

This would be:

char** arr = malloc(sizeof(*arr) * NUMBER_OF_WORDS_IN_INPUT);

arr[0] = malloc(NUMBER_OF_CHARACTERS_IN_WORD0 + 1);
arr[1] = malloc(NUMBER_OF_CHARACTERS_IN_WORD1 + 1);
...
arr[NUMBER_OF_WORDS_IN_INPUT - 1] = malloc(NUMBER_OF_CHARACTERS_IN_LAST_WORD + 1);

Then you can store characters into arr using the syntax

arr[i][j] = SOME_CHARACTER;

without segfaults. (It is of cause required that i and j is within bounds of the allocation).

Upvotes: 2

Adrien
Adrien

Reputation: 497

The inner while loop need to end if s[i] is NULL : while (s[i] != c && s[i] != '\0')

You check for s[i + 1] in your if statement but you continue looping.

Also you are allocating way more bytes than necessary, you could have a buffer of same size of your input string, and when the delimiter or NULL is found, you allocate a new row in your array of the needed size and copy the buffer into it.

Upvotes: 0

Related Questions