array, string, pointer manipulation in C

Question

Supposedly there is a bug in this code, but it runs fine and with an output that I expect ("hello world"). Is there a problem with return str?

#include <string.h>

char* example(){
    // your code goes here
    char str[12];
    strncpy(str, "hello world", 11);
    str[11] = 0;
    printf("%s\n",str);
    return str;
}

int main() {
    char * check = example();
    return 0;
}

Output:

Success time: 0 memory: 2248 signal:0
hello world

Are you looking for the `null byte '\0'`? In any case, you don't have to manually set the null byte with strncpy. — C.B., Jul 31 '14 at 02:25
strncpy you do @C.B. it will stop when it copes n characters and may not set the null terminator. In this case it definitely doesn't. — Keith Nicholas, Jul 31 '14 at 02:27
@KeithNicholas only if source is longer than num bytes to copy, right? — C.B., Jul 31 '14 at 02:28
@C.B, right, like he has. 11 is the number of characters, so he misses copying the 0 — Keith Nicholas, Jul 31 '14 at 02:30
The code now null terminates the string; it may or may not always have done so. However, using `strncpy()` followed by an assignment is a bit clumsy, especially with the repeated 11s. — Jonathan Leffler, Jul 31 '14 at 02:33

Fiddling Bits · Accepted Answer · 2014-08-03T05:01:37.687

9

Big problem!

   return str;

str is a local variable. You must not pass its address to the caller because the contents of what it's pointing to are not guaranteed to be what you assigned to it in example. This wouldn't be a problem if you defined str to be static:

static char str[12];

Edit 1

You can also malloc memory for str:

char *str = malloc(12);

If you choose this method, you must remember to free the dynamically allocated memory to prevent a memory leak.

Another method is to make str global:

char str[12];

char *example(void)
{
    ...
}

It is generally best to keep the scope of variables as limited as possible, however.

Edit 2

Still another method is to have the caller allocate memory for the string. The following is an example:

void example(char *str, size_t length)
{
    strncpy(str, "hello world", length);
    str[length - 1] = '\0'; // Guarantee string '\0' terminated
}

int main(void)
{
    char str[12];
    example(str, sizeof(str));
    printf("%s\n", str);

    exit(EXIT_SUCCESS);
}

The problem with this method is that, generally, the caller does not know the length of the string to be returned.

edited Aug 03 '14 at 05:01

answered Jul 31 '14 at 02:27

Fiddling Bits

8,712
3
28
46

correct, but not sure I'd recommend static though. – Keith Nicholas Jul 31 '14 at 02:28
1

@KeithNicholas Would you recommend `malloc`ing it or making it global? – Fiddling Bits Jul 31 '14 at 02:29
I'd not reccomend global, as its basically the same as static, I'd either pass in a char* which then gets populated, and makes allocation a problem seperate from the function, or return a malloc'd char* – Keith Nicholas Jul 31 '14 at 02:32
1

Making it static means that you can only usefully call the function once; thereafter, any other calls will share the return value. Here, the string returned is constant, so it won't matter much, but most function return varying values. For that, and for thread-safety, a static variable is undesirable. It is often best to have the calling code provide the space (the best design usually takes a pointer and the maximum length). Making the function use `malloc()` also works but places the onus on the calling code to release the allocated memory (but `strdup()` does precisely that. – Jonathan Leffler Jul 31 '14 at 02:34
Usually the function producing a string knows how long the result is, whereas the caller doesn't, so it's better to return malloced memory rather than have the caller provide it. In either case, the caller is responsible for releasing the memory, so that's not an issue. Note that malloc itself requires the caller to free the result at some point ... there's no reason why your own functions can't have the same requirement. Also, strncpy is error-prone and generally sucks -- it not only doesn't NUL-terminate, but NUL-fills the buffer, which can be inefficient. Best to avoid it. – Jim Balter Jul 31 '14 at 03:11
`strncpy` is nearly always a bad idea; `strcpy` or `sprintf` or `snprintf` are better because they null-terminate their output. – M.M Jul 31 '14 at 03:40

score 1 · Answer 2 · answered Jul 31 '14 at 02:29

The bug is that the example() function returns a pointer to the local array str. When a function returns, all its local variables are no longer valid, and using pointers to them is undefined behavior.

It's apparently working because the main() function never actually does anything with the returned pointer. If you put

printf("%s", check);

in main() you would probably get garbage output there.

array, string, pointer manipulation in C

2 Answers2