Modifying a string array inside of a function

Question

The following code example, doesn't print the strings test1 - test5 contained in array in the main() function

however it works inside the make() function

I am certain the answer is simple, how would I produce the desired result?

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

#define ELEMENTS 4

void make(char ***array) {

char p2[] = "test1 test2 test3 test4 test5";
char* token = strtok(p2, " ");
int i = 0;
while (token) 
{

    (*array)[i]= token;
    token = strtok(NULL, " ");
    i++;
}

printf("%s\n",(*array)[0]);
printf("%s\n",(*array)[1]);
printf("%s\n",(*array)[2]);
printf("%s\n",(*array)[3]);
printf("%s\n",(*array)[4]);

}

int main(int argc, char **argv) {
char **array;
make(&array);

int i;
for (i = 0; i < ELEMENTS; ++i) {
    printf("%s\n", array[i]);
}

return 0;
}

This code compiles without error or warning and produces the following output:

test1
test2
test3
test4
test5
yf�


���

my expected result was to have test1 - test5 printed twice, once inside the make() function and once in the main() function

as a side note, this is only my second post to stackoverflow, this is modified code from my first question Passing a string array to a function in C

Answer 1

char p2[] = "test1 test2 test3 test4 test5";

defines the string p2 on the stack of make() function. strtok() only returns pointers to the same array, which becomes invalid when make() returns.

It's the same as this:

char * foo()
{
    char array[] = "hello";
    printf("%s\n", array); // works fine
    return array;
}

void main()
{
    char * array = foo(); // just a pointer to invalid data
    printf("%s\n", array); // FAIL
}

How to do it correctly

There are basically two ways to return strings in C.

Either by filling in a buffer:

void fill_buf(char * buf, size_t len)
{
    char string[] = "hello";
    snprintf(buf, len, "%s", string);
}

void main()
{
    char buffer[25];
    fill_buf(buffer, sizeof(buffer));
    printf("%s\n", buffer);
}

Or by returning malloc'd string:

char * get_malloced_or_null()
{
    char my_string[] = "hello";
    char * copied_pointer = strdup(my_string); // might fail and return NULL
    return copied_pointer;
}

void main()
{
    char * string = get_malloced_or_null();
    if (string == NULL) { return; }
    printf("%s\n", string);
    free(string);
}

The advantage of the second approach is that you can use it with arbitrarily long strings. On the other hand, you need to check for allocation failures, and you have to free memory after you're done with it.

Answer 2

This code compiles cleanly, and produces the desired result, Thanks to all that helped me understand pointers. now i'll work on freeing the memory I allocated to the array

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

#define ELEMENTS 5

void make(char ***array) {

char p2[] = "test1 test2 test3 test4 test5";
char* token = strtok(p2, " ");
int i = 0;
while (token) 
{

    (*array)[i] = malloc(strlen(token) + 1);
    strcpy((*array)[i], token);
    token = strtok(NULL, " ");
    i++;
}

}

int main(int argc, char **argv) {
char **array;
make(&array);

int i;
for (i = 0; i < ELEMENTS; ++i) {
    printf("%s\n", array[i]);
}

return 0;
}

Answer 3

It works with simply defining p2[] static:

static char p2[] = "test1 test2 test3 test4 test5";

also, ELEMENTS should be 5, and the declaration of array in main is a simple "pointer to pointer", it should be "array of pointers":

char *array[ELEMENTS];

then you'll get enough room for your 5 strings.

EDIT: working with an array of pointers simplfies the make() function:

void make(char **array) {
    static char p2[] = "test1 test2 test3 test4 test5";
    char *token = strtok(p2, " ");
    int i = 0;
    while (token) {
        array[i] = token;
        token = strtok(NULL, " ");
        i++;
    }
    for (i = 0; i < ELEMENTS; ++i)
        printf("%d: %s\n", i, array[i]);
}

int main(int argc, char **argv)
{
    char *array[ELEMENTS];
    make(array);
    /* ... */
}

Answer 4

In addition of the above answer;

You have to allocate memory for the char ***array at the beginning the make() function

*array = malloc(ELEMENTS * sizeof(char *));

and ELEMENTS should be define as 5 and not as 4

#define ELEMENTS 5

Answer 5

In the make() function, you are parsing a local variable. As such, the strtok() returns stack address (and you array will reference a content in the stack that could be overwritten any time). Once the function make() is completed, those pointers will refer to anything in your stack and your array content may be any thing.

You may fix this as follow:

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

#define ELEMENTS 4

void make(char ***array)
{
    char p2[] = "test1 test2 test3 test4 test5";
    char* token = strtok(p2, " ");
    int i = 0;
    while (token) 
    {
        (*array)[i]= strdup(token);
        token = strtok(NULL, " ");
        i++;
    }

    printf("%s\n",(*array)[0]);
    printf("%s\n",(*array)[1]);
    printf("%s\n",(*array)[2]);
    printf("%s\n",(*array)[3]);
    printf("%s\n",(*array)[4]);

}

int main(int argc, char **argv)
{
    char **array;
    make(&array);

    int i;
    for (i = 0; i < ELEMENTS; ++i) {
        printf("%s\n", array[i]);
    }

    return 0;
}