1

Need help in getting const char* array from function so elements can be printed in main.

main:

const char* values[3];
strings_to_array();

printf("%s\n", values[1]);
printf("%s\n", values[2]);

function:

const char* strings_to_array()
{
    char one_str[16];
    char two_str[16];
    char three_str[16];

    strcpy(one_str, "one");
    strcpy(two_str, "two");
    strcpy(three_str, "three");

    const char* values[] = {one_str, two_str, three_str};
    return values;
}

What is incorrect here and how to get values to main?

alk
  • 69,737
  • 10
  • 105
  • 255
Wine Too
  • 4,515
  • 22
  • 83
  • 137
  • The `values` in `strings_to_array` is totally different from the `values` in `main`. You can't just expect the `strings_to_array` will modify the `values` in `main`. – leonhart Jun 07 '13 at 10:40

6 Answers6

9

The main problem with this code is that: functions in C should not return pointers to local variables as they are stored on the stack, which means they are not available once the function returns.

So this line:

const char* values[] = {one_str, two_str, three_str};

Can be replaced with:

const char** values = malloc(3*sizeof(char *));
values[0] = strdup(one_str);
values[1] = strdup(two_str);
values[2] = strdup(three_str);

The full working code of the above example:

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

const char** strings_to_array()
{
    char one_str[16];
    char two_str[16];
    char three_str[16];

    strcpy(one_str, "one");
    strcpy(two_str, "two");
    strcpy(three_str, "three");

    const char** values = malloc(3*sizeof(char *));
    values[0] = strdup(one_str);
    values[1] = strdup(two_str);
    values[2] = strdup(three_str);
    return values;
}

int main() {
  const char** values = strings_to_array();

  printf("%s\n", values[1]);
  printf("%s\n", values[2]);

  free((void *)values[0]);
  free((void *)values[1]);
  free((void *)values[2]);
  free(values);      

  return 0;
}
Adam Siemion
  • 15,569
  • 7
  • 58
  • 92
  • +1 on the hope that you will add appropriate calls to "free" and a return 0; to main. – wolfgang Jun 07 '13 at 10:43
  • That mean I should free all those variables in main after usage? – Wine Too Jun 07 '13 at 10:43
  • @user973238 Yes, you SHOULD, but as long as your program is going to end right away anyway, you can get away without doing it as the OS is going to clean up after you. – wolfgang Jun 07 '13 at 10:48
  • The call to malloc() allocates the wrong size of memory. The way you do it (`strdup()`ing the "strings") you need no more then `3*sizeof(char*)`! If all those three strings were `char[3]` for example this code would gloriously crash. – alk Jun 07 '13 at 10:49
  • @alk Good spot, that was because in the first version of this program I allocated one array. Fixed it now. Thanks – Adam Siemion Jun 07 '13 at 10:53
  • @user973238 as wolgang has written in case of this application the OS will do a cleanup but since it is a good habit in C to always free the allocated memory I have added calls to `free`, thanks. – Adam Siemion Jun 07 '13 at 10:54
  • Btw: At least in C there is no need to cast when calling `free()`. – alk Jun 07 '13 at 10:54
  • @alk Yes, it is just to avoid `warning: passing argument 1 of 'free' discards qualifiers from pointer target type` – Adam Siemion Jun 07 '13 at 10:55
  • I feel this casting away a warning clearly points out a design flaw. You might like to have a look at my answer. – alk Jun 07 '13 at 11:34
  • @alk I agree there is no point in creating these local variables like `one_str`, but I did not want to change too much in the original code. – Adam Siemion Jun 07 '13 at 11:46
1
  1. It is wrong syntactically because you declared return type as const char* and you are trying to return const char**.

  2. It is wrong semantically because you are trying to return a pointer to array allocated on stack.

Grzegorz Żur
  • 47,257
  • 14
  • 109
  • 105
0

Nothing initializes the array of character pointers named "values" in main. What is that function supposed to do? It does nothing, and its return value is ignored in main.

DarenW
  • 16,549
  • 7
  • 63
  • 102
0

The variables one_str, two_str, three_str, and values are local variables of the 'strings_to_array' function. They have no valid existence outside this function.

By returning 'values' (ie: a pointer to a local variable), you return a pointer to an invalid memory location.

user803422
  • 2,636
  • 2
  • 18
  • 36
0

One way is to declare your variables as static since you are going to access them outside of that function. It just means that memory for those are not lost once you exit the function. Also, you should be returning a char ** instead of char *.

const char** strings_to_array()
{
    static char one_str[16];
    static char two_str[16];
    static char three_str[16];

    strcpy(one_str, "one");
    strcpy(two_str, "two");
    strcpy(three_str, "three");

    static const char* values[] = {one_str, two_str, three_str};
    return values;
}
n3rd4n1
  • 371
  • 2
  • 8
  • What after usage with static variables? Can they be freed to? – Wine Too Jun 07 '13 at 10:51
  • They shouldn't be freed. Their lifetime is the same as the process. [This](http://stackoverflow.com/questions/408670/stack-static-and-heap-in-c) is a good reference, I suppose. – n3rd4n1 Jun 07 '13 at 11:32
0

As you are dealing with literals ("one", ...) you could simply assign their addresses.

main:

void string_to_array(const char **);

const char * values[3] = {NULL};
strings_to_array(values);

printf("%s\n", values[0]);    
printf("%s\n", values[1]);
printf("%s\n", values[2]);

function:

void strings_to_array(const char ** values)
{
  values[0] = "one";
  values[1] = "two";
  values[2] = "three";
}

And if you really need it as a function returning a reference to the array do it this way:

const char ** strings_to_array2(const char ** values)
{
  values[0] = "one";
  values[1] = "two";
  values[2] = "three";

  return values;
}

This keeps you from doing the ugly call to free() on const char *s.

alk
  • 69,737
  • 10
  • 105
  • 255
  • But can values in _to_array2 be filled like this: values[3] = {"one", "two", "three"};? – Wine Too Jun 07 '13 at 12:27
  • @user973238: No, for two reasons: 1 `values[3]` is **one** entry in the array `values` namely the third. 2 The construct ` = {}` with ` = , , ..., ` is allowed for initialisation only, not for assignments. – alk Jun 07 '13 at 12:37