Array of char* in C

Question

What I want my program to do is to read an input text from the user, tokenize that string with the whitespace being the delimiter and store each of the tokens int an array of char* which will then be returned.

here is a snippet of code that I am trying to make it work correctly:

typedef char* String;
String* split(char* cmd)
{
    char* param;
    char tmp[128];
    String* result = (String*) malloc(10*sizeof(String));
    memset(result,NULL,10);

    strcpy(tmp,cmd);

    param = strtok(tmp," ");

    int index = 0;
    while(param && index < sizeof(result) / sizeof(*result))
    {

        result[index] = (char*) malloc(strlen(param));
        strcpy(result[index],param);

        param = strtok(NULL," ");
        index++;
    }

}

Where cmd is the string that I am tokenizing and result is the array that will contain each token.

This snippet causes errors when trying to iterate through the returned result using a simple for loop (A segmentation fault arises)

String* splittedCmd = split(command);

int i;
for(i=0;i<10;i++)
{
    if(splittedCmd[i] != NULL)
        printf("%s\n",splittedCmd[i]);
}

Think about local variables and lifetime. Think about dynamic allocation, and why you don't have it. Think about using strncpy instead. Think about marking this as "homework". And think about switching to C++. — Kerrek SB

Seth Carnegie Seth Carnegie · Accepted Answer · 2012-01-23T02:29:54

There are several things wrong here.

First and most obvious, you are returning result which is an array (but decays to a pointer to an array) that is allocated on the stack in the function so it is reclaimed when the function returns. You need to dynamically allocate the array (and rely on the caller to free it):

String *result = malloc(10 * sizeof(String));

Also, your condition to stop the while loop:

if(index == sizeof(result))

Will let the loop go until index is 40 (if char* is 4 bytes on your platform) because sizeof returns the size of the operand in bytes, not array elements, so sizeof(result) is (again, platform dependent) 40. This is obviously beyond the bounds of the array.

If you were still using a local array instead of malloc, you could change that to

if (index == sizeof(result) / sizeof(*result))

However, you can't do that now because result is only a pointer instead of an array and sizeof(result) will always be the size of a pointer on your platform.

You can just remove that if completely and change the while condition to

while (param && index < 10)

Which makes sure that param is not NULL and also that index is less than 10. You should consider making a #define or a const int or something for the size of the array and use that instead of using a magic number.

You also need to change

memset(result,NULL,10);

to

memset(result,NULL, sizeof(String) * 10);

Because if you don't, memset is only setting the first 10 bytes of the memory pointed to be result to 0, instead of the whole thing, because it takes the number in bytes, not array elements.

Array of char* in C

3 Answers