1

Is this a proper thing to do in C ? 

char *func1() {
    char *str[3] = { "so", "is", "amazing" }; 

    return str[1]; 
}

The char *func1() returns an pointer, pointing to a location in memory where the string is stored. But when the func1() returns won't the str array go out of scope and am I not returning a pointer to an object that does not exist? A dangling pointer (is this the right name?)

So I have two solutions make str global so that it never goes out of scope and the pointer always pointer to a valid memory address, but that seems dirty.

The other solutions

char *func2() {
    char *str[3] = { "so", "is", "amazing" };
    int str1Len = strlen(str[1]); 
    char *ret = (char *)malloc(str1Len) ; // create a block of mem to hold the str[1]. 
    strncpy(ret, str[1], str1Len);
    return ret; 
}

Is this correct ? What is the proper way to do this func1() or func2()? Or is there a better way?

I have been using func1() for some time and it has not created any problems for me, does this mean that func1() is doing the right thing? and func2() is unnecessary?

Improve this question
7
  • 1
    char * str[3] = { "so" , "is" ,"amazing" } ; is declared within the function and ceases to exists when the function returns. (it is local to the function, and that memory is released when the function returns -- so the memory pointed to by str[1] no longer exists) You must either pass str to the function as a parameter, or allocate storage for str with malloc within the function and return a pointer to the new block of memory. Commented Feb 7, 2016 at 22:30
  • @DavidC.Rankin Yeah this is what I though too , but C CPP and there rules are simply too crazy. See this stackoverflow.com/questions/349025/… Commented Feb 7, 2016 at 22:45
  • @DavidC.Rankin: the str array itself has automatic storage, but its elements being pointers to string literals, that have static storage. It's OK to return one of them, pointers are returned by value. Commented Feb 7, 2016 at 22:46
  • @nnrales: I'm afraid DavidC.Rankin is mistaken. Your example is simple and if your actual code uses arrays of pointers to string literals, you can return elements from them. Commented Feb 7, 2016 at 22:48
  • 2
    If str does not change, make it static const char * const str[] = .... That qualifies both, the array and the char [] it points to const. The static makes it permanent and the rest makes it startup-time initialised. Your current version will setup the array for every call. Commented Feb 7, 2016 at 22:49

3 Answers 3

Reset to default
3

In your first function, there is no problem returning str[1] as it is not pointing to a local variable, it is a pointer to a string literal. Note that string literals should be declared const:

const char *func1(void) {
    const char *str[3] = { "so", "is", "amazing" };
    return str[1]; 
}

Your second function returns a pointer to allocated space. This pointer will need to be freed at some point. The allocation is incorrect, you should allocate 1 extra byte for the final '\0'. You can use strcpy to copy the string to the allocated space, or simply use strdup() that does both:

char *func2(void) {
    const char *str[3] = { "so", "is", "amazing" };
    int size = strlen(str[1]) + 1; 
    char *ret = malloc(size);
    return strcpy(ret, str[1]);
}

Or simply:

char *func2(void) {
    const char *str[3] = { "so", "is", "amazing" };
    return strdup(str[1]);
}

NEVER use strncpy, it does not do what you think it does. It is very error prone for both the programmer and whoever will read your code.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Sorry for the stupid question but isn't the string literal a local variable ?
@nnrales It has static storage duration, it doesn't matter if it's a local variable. And it's a smart question if anything.
@iharob thank you the static storage duration page . I did not know anything about it.
1

  1. Is this a proper thing to do in C ?

    No, it's not! You need to create a copy of the local variable in general, although in this case the pointers point to string literals so it would work.

  2. You are allocating and copying the string wrong, you should do this instead

    size_t size =  strlen(str[1]) + 1; 
char *result = malloc(size) ; // create a block of mem to hold the str[1]. 
if (result != NULL)
    memcpy(ret, str[1], size);
Improve this answer

9 Comments

str[1] is not pointing to a local variable, it is a pointer to a string literal. There is not problem returning it, except maybe for constness.
Why do you prefer memcpy to strncpy ? So it is alright if I write a program which uses func1() ? Why is it so ? I though when a functions returns every object created on the stack is technically de-allocated ?
@chqrlie I said that, I said in general it's not a good idea but since they are string literals.
strncpy() is not for this, it will try to detect the end of the string before length is reached (it copies at most length bytes), memcpy() will copy the exact number of bytes you tell it to copy. Since you have used strlen() then the '\0' better be in the source string or undefined behavior will occur. So you are sure that it is there then copy length bytes that's all. But also, on ocasions it will not copy the '\0' terminator so as a general rule do not use strncpy().
strncpy is not advisable anywhere.
|
0

While it is acceptable to return pointers to the string literals, buit not to a local pointer, it might be the wrong approach.

If str does not change, make it static const char * const str[] = .... That qualifies both, the array and the char [] (which is each string literal) it points to const. The static makes it permanent, but with still local scope. It is setup once before your program code starts. In contrast, a non-static version will setup the array for every call.

Note that you have to return a const char * to maintain const-correctness. I strongly recommend that; if you cannot, omit the first const only.

Note: Do not cast the result of malloc & friends (or void * in general) in C.

Improve this answer

5 Comments

Thank you , I implemented what you said but to return it as a char * I had to drop the const. My Prof, tells me to always cast the result of malloc and void * into the type that I know it is . Why do you say other wise ?
@nnrales: Sorry, but your prof is blatanic wrong! He might want to read the C standard. Also there is a thread here already about that subject, see the info-page for this and more. Maybe he should learn C is not C++ (in C++ you have to) and shall not use a C++ compiler to compile C code. Casts inhibit type-checking,as they tell your compiler to "shut up" and not to warn you. Only use a cast if you 1) have to 2) really understand all implications and 3) accept them completely.
@nnrales: About const-correctness: I strongly recommend to keep that and - as I wrote - correct the other code. const-correctness is an important measure to make your code safer and help your compiler tell you if something is fishy with your code.
Thanks for your tip. My array is quite large and reallocating it each time the function is called , will be expensive.
For a large array, it might be better to make it file-local. Just move it as-is outside the function (leave it static). That for readability and to keep your functions source code footprint small.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.