0

I am trying to pass a array of pointers to string to a function where I need to set the values. In the passing function I do not know the number of strings I will get, the called function is calling some other function which returns list of strings.

Sample code below:

int main() {
    char** list;
    create(list);
}

int create(char **array) {

    char* str[] = { "hello", "dear" };
    int len;
    int i = 0;
    for (i = 0; i < 2; i++) {
        len = strlen(str[i]);
        printf("%d\n", len);
        *(array + i) = (char*) malloc(len * sizeof(char*));
        strcpy(*(array + i), str[i]);
        i++;

    }
    return 1;
}

This gives me segmentation fault.

What wrong am I doing here. Please help.

Thanks

EDIT Updated code from below comments:

int main() {
    char** list;
    create(list);
    int i = 0;
    for (i = 0; i < 2; i++) {

        printf("%s\n", list[i]); // segmentation fault
    }
}

int create(char **array) {

    char* str[] = { "hello", "dear" };
    int len;
    int i = 0;
    array = malloc(2 * sizeof(char*));
    for (i = 0; i < 2; i++) {
        len = strlen(str[i]);
        printf("%d\n", len);
        *(array + i) = (char*) malloc(len * sizeof(char));
        strcpy(*(array + i), str[i]);
        printf("%s\n", array[i]); // this prints
    }
    return 1;
}

Now getting segmentation fault in main while printing the list.

Actual code where I am reading the strings

int i;
for ( i=0; i<reply->elements; i++ )
{
printf( "Result: %d---%s\n", i,reply->element[i]->str );
*array[i] = (char*)malloc(strlen(reply->element[i]->str));
printf("***");
strcpy(array[i],reply->element[i]->str);
printf( "Array[%d]: %s\n", i,array[i] );
}
Jardanian
  • 711
  • 1
  • 6
  • 11
  • possible duplicate of [malloc-ating multidimensional array in function](http://stackoverflow.com/questions/5845686/malloc-ating-multidimensional-array-in-function) – n. m. could be an AI Sep 16 '15 at 15:26

4 Answers4

1

  1. You need to allocate some space for list or undefined behavior occurs:

    char* list[2];

  2. You increment i twice; therefore, remove the i++ from the bottom of the for loop.

Minor notes:

  • refer to string literals as const char*
  • use array[i] instead of *(array + i)
  • don't cast the result of malloc

  • malloc allocates too much space as you allocate len char*s, even though you need just chars. Also, as @CoolGuy noted, you need one extra byte for the null byte. Replace the allocation with

    array[i] = malloc(len * sizeof(char) + sizeof(char));

    or

    array[i] = malloc(len + 1);

  • call free after malloc

  • you assign 0 twice to i; remove the initialization

Community
  • 1
  • 1
cadaniluk
  • 15,027
  • 2
  • 39
  • 67
  • Thanks. I do not know the size in main so cannot give it there. I get what you are saying about passing pointer but is not pointer what I am passing? I understand that I am passing address to first location. – Jardanian Sep 16 '15 at 15:31
  • @Jardanian Why don't you know the size? Viewing your code it seems you do know it as the `for` loop will definitely loop twice. – cadaniluk Sep 16 '15 at 15:34
  • This is just a sample code which I prepared to show what I need. Actual code is bigger. – Jardanian Sep 16 '15 at 15:39
  • You forgot to allocate space for the NUL-terminator. – Spikatrix Sep 16 '15 at 15:40
  • From the calling function I'll pass the list and an pointer to integer which will be updated with the size. – Jardanian Sep 16 '15 at 15:40
  • @Jardanian Then I recommend posting the real code. How exactly do you read the strings? This is important as it's tightly related to how to determine the number of strings. – cadaniluk Sep 16 '15 at 15:48
1

You correctly alloc memory for the individual strings, but fail to alloc some for the array itself.

You should use:

int main() {
    char* list[8] = {0}; /* initialize pointers to NULL */
    create(list);

    /* free allocated memory - free(NULL) is legal and is a noop */
    for (i=0; i<sizeof(list)/sizeof(list[0]); i++) free(list[i]);
    return 0;  /* never return random value from main */
}

And you should remove the i++ at the end of the loop in function create because it leads to a double increment.

Alternatively you could alloc the array itself in the function create:

int create(char ***array) {

    char* str[] = { "hello", "dear" };
    int len;
    int i = 0;
    *array = malloc(1 + sizeof(str)/sizeof(str[0]));
    for (i = 0; i < 2; i++) {
        len = strlen(str[i]) + 1;
        printf("%d\n", len);
        (*array)[i] = malloc(len * sizeof(char*));
        strcpy((*array)[i], str[i]);
    }
    (*array)[i] = NULL;
    return i;
}

int main() {
    char** list;
    create(&list);
}

In above code, the length of the array is the return value from create, and the last element of list is a NULL (in the same logic as argc/argv).

Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252
  • Thanks. I do not know the size in main so I have to allocate in create(). I did that and now create is fine but when I try to print values in main it gives seg fault. – Jardanian Sep 16 '15 at 15:29
  • Thanks. This works. But what' wrong with the edited code I posted and what is the use of **(*array)[i] = NULL;** – Jardanian Sep 16 '15 at 15:46
  • OK. Got it. It's the null terminator for the array. Correct? – Jardanian Sep 16 '15 at 15:57
  • @Jardanian: As I said, I wanted the array to be terminated with a NULL pointer, as is the argv array. But this is absolutely optional since the length is already passed in return value. – Serge Ballesta Sep 16 '15 at 16:02
  • @Jardanian: In your code, you just pass `list` to the function. So when you assign the malloced memory to it, you just change **a local copy** and not the caller variable. That's the reason why I pass the address of `list`. – Serge Ballesta Sep 16 '15 at 16:05
  • Thanks a lot for you help. May I request you to please explain just two things. 1.) How will we write (*array)[i] as just pointer notation, is it **(array+i)? 2.) What did I pass as char **, I thought this passes the address to first location. – Jardanian Sep 16 '15 at 16:06
  • Ok. If I had created like **char *a[2]** and passed **create(a)** then would it be the address or still I require to do **create(&a)**. Sorry, but I am trying to understand the pointers but getting lost always. – Jardanian Sep 16 '15 at 16:12
  • @Jardanian: C always pass by value. So you can *use* what was passed but not *change* it. If you define `char *a[2]` and then pass `create(a)`, you can **use** `a` in `a[i] = ...`, but you must never have `a = ...` in a function `create(... a)`. – Serge Ballesta Sep 16 '15 at 17:09
  • Thanks. Some say pass by ref and some say pass by val. I'll read more but how this happens [change value of char array in called function](https://ideone.com/NVS5nu) – Jardanian Sep 16 '15 at 17:50
0

You allocate two arrays (char*) to store the strings "hello" and "dear" but does not allocate the array (char**) containing those two string array.

Richard Dally
  • 1,432
  • 2
  • 21
  • 38
0

I would suggest you to change declaration of function create to this -

int create(char ***array);

And call it like this -

create(&list);

In function create allocate memory like this -

    *array = malloc(2 * sizeof(char*));
    for (i = 0; i < 2; i++) 
        {
            len = strlen(str[i]);
            printf("%d\n", len);
            (*array)[i] =malloc(len * sizeof(char*)+1);
            strcpy((*array)[i], str[i]);
        }

And do the printing as you do in main.

Note - free memory that you allocate.

And you should declare len as type size_t -> size_t len; and print it with %zu specifier in printf .

See working code here -https://ideone.com/GX2k9T

ameyCU
  • 16,489
  • 2
  • 26
  • 41