[SOLVED] How to resolve error displayed by Valgrind “Conditional jump or move depends on uninitialised value(s)”


This Content is from Stack Overflow. Question asked by Tesakt

I am writing program in C to load dictionary and then check provided file for misspellings. Function to load dictionary uses hash bucket and chaining rule to resolve collisions. Chains are made from linked lists:

#define LENGTH 45

    typedef struct node
    char word[LENGTH + 1];
    struct node *next;

Everything is working perfectly, except when I check the code with Valgrind. There is no memory leaks, but there seems to be error in using uninitialized value:

Conditional jump or move depends on uninitialised value(s)
    by 0x109B8F: load (dictionary.c:111)
    by 0x10929B: main (speller.c:40)
  Uninitialised value was created by a heap allocation
    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x109B21: load (dictionary.c:96)
    by 0x10929B: main (speller.c:40)

I repeatedly debugged code narrowing down source of this error to my function responsible for loading dictionary to heap. To be more exact problem seem to be in creating temporary pointer for new node and then trying to access word variable inside node even when the word is copied from another char table and initialized:

node *temp = malloc(sizeof(node)); // Error source (line 96)

if (temp == NULL)
   return false;

temp->next = NULL;

for (int i = 0; word_buffer[i] != ''; i++)
    temp->word[i] = word_buffer[i];

int key = hash(temp->word); // Error in using temp->word, not initialised (line 111)

How can I change these lines of code to resolve Valgrind error?
Can you explains source of this error? I only recently started my programming journey and want to know to avoid this error in future


The code shown fails to add a null terminator to temp->word. Supposing that hash expects to receive a null-terminated string, the lack of a null terminator would cause hash to read uninitialized elements in temp->word.

This could be fixed by changing:

for (int i = 0; word_buffer[i] != '\0'; i++)
    temp->word[i] = word_buffer[i];


int i;
for (i = 0; word_buffer[i] != '\0'; i++)
    temp->word[i] = word_buffer[i];
temp->word[i] = '\0';

although it should be noted the code shown has no guard against exceeding the length of temp->word array.

This Question was asked in StackOverflow by Tesakt and Answered by Eric Postpischil It is licensed under the terms of CC BY-SA 2.5. - CC BY-SA 3.0. - CC BY-SA 4.0.

people found this article helpful. What about you?