Conditional jump or move depends on uninitialised value

Question

Please review the following code:

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

int main(void) 
{
    char *some_text = "THIS IS SOME ARBITRARY TEXT FOR TESTING THE STRLEN AND COPY FUNCTIONS";

    char *test = malloc(strlen(some_text) + 1);
    strncpy(test, some_text, strlen(some_text));
    strcat(test, "\0");

    printf("%s\n", test);
    if (test != NULL) free (test);
}

1. create a char* some_text with constant string;

2. create a char*test with and allocate memory the size of some_text

3. copy the content of some_text to test

4. concatenate the termination character to test

5. print out the test

6. free the test pointer

If I run this with valgrind, it says:

==31176== Conditional jump or move depends on uninitialised value(s)
==31176==    at 0x4008667: __GI_strlen (mc_replace_strmem.c:404)
==31176==    by 0x861DD4: puts (in /lib/libc-2.12.so)
==31176==    by 0x80484DF: main (strlen_and_copy.c:13)

The line mentioned in valgrind output strlen_and_copy.c:13 is below:

printf("%s\n", test);

Is it avoidable or should it be ignored? What is wrong with my code above? Someone please explain.

Answer 1

Some general remarks to your code:

1. You should store the string length as a variable rather than calling it multiple times.
2. If you do strncpy(test, some_text, strlen(some_text) + 1) you can get rid of the strcat as strncpy will padd the remainder with \0 if src is less than n characters.
3. You don't need to test for NULL before free
   • a) because you'd get a segfault anyway before that line of code if it were NULL
   • b) because the standard guarantees that it is safe to call free(NULL)

Answer 2

First string you pass to strcat is not null-terminated, which is incorrect (it takes two null-terminated byte strings). Actually it is null-terminated, but only because malloc returns pointer to zeroed memory on your system. Anyway, after call to strncpy you have a string with last byte uninitialized.

This code

strcat(test, "\0");

does nothing ("\0" and "" are equal).

To make it correct you can write

strcpy(test, some_text);
// without strcat

or

strncpy(test, some_text, strlen(some_text) + 1)

or

strncpy(test, some_text, strlen(some_text));
test[strlen(some_text)] = 0;