CODEWITHSUNDEEP
carrayschar
user1162954
user1162954

Reputation: 267

Array of char* in C

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]);
}

Upvotes: 2

Views: 467

Answers (3)

Seth Carnegie
Seth Carnegie

Reputation: 75130

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.

Upvotes: 2

Manish Burman
Manish Burman

Reputation: 3079

Just a hint - why don't you try to use strtok? It would simplify things greatly.

Upvotes: 1

asaelr
asaelr

Reputation: 5456

You should not return a pointer to local variable. After the function split return, the memory address of result may contain garbage, and therefore you refer to invalid pointer when you try to print it.

Upvotes: 0

Related Questions