Uninitialized value from valgrind

Question

==2630== Conditional jump or move depends on uninitialised value(s)
==2630==    at 0x4E82D71: vfprintf (in /usr/lib64/libc-2.21.so)
==2630==    by 0x4E88E78: printf (in /usr/lib64/libc-2.21.so)
==2630==    by 0x400C0C: searchWord (T9.c:91)
==2630==    by 0x400A0A: main (T9.c:40) 

==2114==  Uninitialised value was created by a heap allocation
==2114==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==2114==    by 0x400FD1: newStr (trie_node.c:125)
==2114==    by 0x400F8C: create_trie (trie_node.c:117)
==2114==    by 0x4009D5: main (T9.c:37)

I have error message above by running trace function in valgrind. I am pretty sure that I have initialized the variable. Here is the struct code:

struct wordList* newStr(char* text) {
    char* word;
    struct wordList* tmp = (struct wordList*)malloc(sizeof(struct wordList));
    word = (char *)malloc(sizeof(char) * strlen(text) + 1);
    strncpy(word, text, strlen(text));
    tmp->str = word;
    tmp->next = NULL;
    return tmp;
}

And the code around T9.c line 91:

struct wordList* cur;
if (cur && invalid == 0 && flag == 0) {
  printf("\t\'%s\'\n", cur->str);
 }

Updates:

I modify the strncpy line from

    strncpy(word, text, strlen(text));

to

word = strncpy(word, text, strlen(text));

This solved the uninitialized problem, however I got new error message that I dont understand:

==3245== Invalid read of size 1
==3245==    at 0x4E82D71: vfprintf (in /usr/lib64/libc-2.21.so)
==3245==    by 0x4E88E78: printf (in /usr/lib64/libc-2.21.so)
==3245==    by 0x400C0C: searchWord (T9.c:91)
==3245==    by 0x400A0A: main (T9.c:40)
==3245==  Address 0x51f7d45 is 0 bytes after a block of size 5 alloc'd
==3245==    at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3245==    by 0x400FC1: newStr (trie_node.c:124)
==3245==    by 0x400F80: create_trie (trie_node.c:117)
==3245==    by 0x4009D5: main (T9.c:37)

You can leave out `sizeof(char)`, since that'll be 1, but if you want to be explicit, put parentheses around `strlen(text) + 1`. Also, don't cast the return value of malloc: `void *` will find its way to your `char *word` by itself. — , Nov 13 '15 at 06:24
There's a safer variant of `strncpy` : it's called `strcpy`. Using it here would have avoided this bug. — M.M, Nov 13 '15 at 07:11
@Big_tboy well, the other two posted answers already pointed it out... — M.M, Nov 13 '15 at 07:25

score 3 · Accepted Answer · answered Nov 13 '15 at 06:23

3

I need to see more code, but while I'm not sure this is exactly what valgrind is complaining about, your code does have a major bug in it:

strncpy(word, text, strlen(text));

You're not null-terminating your string, you're just copying the actual characters. It's particularly funny because there's a function that already takes care of allocating the correct amount of memory and copying strings: strdup.

Also obligatory warning to stop casting the return value of malloc.

answered Nov 13 '15 at 06:23

Blindy

65,249
10
91
131

Null terminate your string. I already mentioned it, please read the reply. – Blindy Nov 13 '15 at 07:05
2

There's no function called `strdup` in C, it is non-standard. So there's no reason to be smug about that one. – Lundin Nov 13 '15 at 07:16
Like Lundin said, there is no `strdup` in `C`. [If you need it, you need to write it yourself](http://stackoverflow.com/questions/33224761/strdup-causing-memory-leaks/33224834#33224834) – Michi Nov 13 '15 at 09:59

skyking · Answer 2 · 2015-11-13T07:06:24.550

You have to look at the complete error (not just the second). As a rule of thumb you should read errors from top to bottom as the subsequent errors may be a consequence of the former (or an explaination of the former). The second message just tells you that the memory allocated by malloc is not initialized, which is as expected - it's just an ammendmend to the first.

The fault lies in when you then use the allocated space without filling it with something useful. The fault lies in the newly posted code, the block pointed to by cur->str is probably uninitialized.

The fault is a bit tricky, since you use strncpy. It will only copy strlen(text) bytes an thereby skipping the null terminator. You've made space for it but it's not copied, consequently the last byte of the allocated buffer is not initialized (the error happens when vfprintf iterates through the string and reaches the byte where the null terminator should be).

To be clear about what strncpy(dst, src, cnt) does. It copies at most cnt bytes (including null terminator), if the src string including the null terminator is no longer than cnt bytes it will copy the full string. Otherwise it will only copy the first cnt bytes of the string and not null terminate dst.

@Big_tboy I meant the data pointed to by `cur->str` is not fully initialized. — skyking, Nov 13 '15 at 06:37
that why i am asking this question, clearly i have initialized this str in my newStr function — Big_t boy, Nov 13 '15 at 06:51
@Big_tboy You have allocated `strlen(text)+1` bytes, but only initialized the first `strlen(text)` bytes. The last byte, which should contain the null terminator is **not** initialized. — skyking, Nov 13 '15 at 07:04

Uninitialized value from valgrind

2 Answers2