Could valgrind complaint about uninitialized values be a false positive?

Question

So I've been teaching myself C, and in the hopes of learning how to properly manage memory from the beginning and write better code, I've been running Valgrind on everything. This has helped me with memory leaks, but I can't seem to get rid of this "Conditional jump or move depends on uninitialised value(s)/Uninitialised value was created by a heap allocation" situation, although I've narrowed it down to this block of code:

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

int main()    
{  
    char* test = (char*) malloc(3);
    strncpy(test, "123", 2);
    printf("%s\n", test);
    free(test);
    return 0;
}

When I run Valgrind with ---track-origins=yes, I get this output:

==91702== Conditional jump or move depends on uninitialised value(s) 
==91702==    at 0x100011507: strlen (mc_replace_strmem.c:282)
==91702==    by 0x1000AA338: puts (in /usr/lib/libSystem.B.dylib)
==91702==    by 0x100000EFA: main (valgrind_test.c:10)
==91702==  Uninitialised value was created by a heap allocation
==91702==    at 0x100010345: malloc (vg_replace_malloc.c:236)
==91702==    by 0x100000EEA: main (valgrind_test.c:8)

This seems like a false positive to me, but I'm not confident enough in my own knowledge to write it off as such. Maybe I'm allocating wrong or using strncpy wrong? I'm not sure.

Thanks in advance

Your string has no terminator, so valgrind is probably right when it complains. — Paul R, Jun 14 '11 at 15:38
Using snprintf here would be better IMHO. snprintf(test, 3, "123"); — puffadder, Jun 14 '11 at 15:41

score 18 · Accepted Answer · answered Jun 14 '11 at 15:37

18

No, this is broken code.

You have a memory block with 3 uninitialized characters. Then you copy "12" into it, and don't terminate. Beware of strncpy().

I quote the documentation:

The strncpy() function is similar, except that not more than n bytes of src are copied. Thus, if there is no null byte among the first n bytes of src, the result will not be null-terminated.

Since there is no termination within the first 2 characters of the source, the destination is not terminated.

answered Jun 14 '11 at 15:37

unwind

391,730
64
469
606

3

It never hurts to add a note: **never use `strncpy`** – R.. GitHub STOP HELPING ICE Jun 14 '11 at 16:01
Seems I didn't read the man page close enough - I thought strncpy would fill the rest of the target string with nulls. That only happens when n is longer than the source string. – pivotal Jun 14 '11 at 17:02
@pivotal: Right. How else, aside from `n`, could it know the length of the target string? @ninjalj: Yes, if anybody still uses them. :-) But it's an obscure enough use that I find it's usually safer to just tell beginners "never" rather than try to explain fixed-size record formats. – R.. GitHub STOP HELPING ICE Jun 15 '11 at 00:13
@R.. What do you suggest as an alternative for extracting substrings from an existing string? – pivotal Jun 15 '11 at 16:03
1

Personally I recommend never using any functions except `memcpy`, and `snprintf` to write strings. The latter can extract substrings of length N quite well with the `%.*s` specifier, and it's 100% safe if you follow a simple correctness pattern in using it. If you used `memcpy` you'd have to take your own precautions with length and null termination. (Incidentally, `strncat` can also do exactly what you want.) – R.. GitHub STOP HELPING ICE Jun 16 '11 at 01:50

score 5 · Answer 2 · answered Jun 14 '11 at 22:01

Idiomatic ways to use strcpy() and strncpy():

If you know that the buffer has space for the string plus the NUL terminator, you can use strcpy(). This will probably use constants, or have checks in code (you should make sure the checks are right).

Otherwise, you can do:

strncpy(dest, src, length);
dest[length - 1] = '\0';

which has the following disadvantages:

it can truncate strings.
it is potentially inefficient, since it always fills length bytes.

There is also OpenBSD's strlcpy().

Any other use of strcpy()/strncpy() is potentially suspicious, and you should look at them carefully.

Bottom line: avoid C string functions for anything moderately complex, try to use some library for dynamically allocated strings. Qmail/Postfix roll their own, GNU has obstacks.

score 1 · Answer 3 · answered Jun 14 '11 at 15:39

1

Your string has no terminator, so valgrind is probably right when it complains. Change:

strncpy(test, "123", 2);

to:

strcpy(test, "12");

answered Jun 14 '11 at 15:39

Paul R

208,748
37
389
560

Could valgrind complaint about uninitialized values be a false positive?

3 Answers3