Please review the following code:

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

int main(void) 

    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.

  • \$\begingroup\$ Is that Valgrind's cryptic way of complaining that you didn't check whether malloc() returned NULL? \$\endgroup\$ Commented Apr 8, 2014 at 8:15

2 Answers 2


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)
  • \$\begingroup\$ Thaks @chrisWue, Actually why this output is coming in valgrind ==31176== at 0x4008667: __GI_strlen (mc_replace_strmem.c:404) ? Still it is availeble \$\endgroup\$
    – Muneer
    Commented Apr 8, 2014 at 8:32
  • \$\begingroup\$ Thaks @chrisWue, Ok. your 2nd point made it success. \$\endgroup\$
    – Muneer
    Commented Apr 8, 2014 at 8:38

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


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


strncpy(test, some_text, strlen(some_text));
test[strlen(some_text)] = 0;
  • 5
    \$\begingroup\$ Good observation that strcat() cannot be used to add a NUL terminator to a string that doesn't already have one. Also a good observation that test is NUL terminated more by luck than by design. However, @ChrisWue's strncpy(test, some_text, strlen(some_text) + 1) is a better solution. \$\endgroup\$ Commented Apr 8, 2014 at 9:12

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.