3

I am having an issue where valgrind is saying that i have a conditional jump or the move depends on initialized values

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

ListNodePtr convertBSTtoLinkedList(TreeNodePtr root)
{
    ListNodePtr newNode;
    ListNodePtr leftLinkedList;
    ListNodePtr rightLinkedList;
    ListNodePtr head;

    if (root == NULL)
    {
        return NULL;
    }

    newNode = malloc(sizeof(struct ListNode));
    newNode->key = root->key;
    head = newNode;

    if(root->left != NULL)
    {
        leftLinkedList = convertBSTtoLinkedList(root->left); 
        head = leftLinkedList;

        ListNodePtr conductor;
        conductor = leftLinkedList;
        while (conductor->next != NULL)
        {
            conductor = conductor->next;
        }

        conductor->next = newNode;

    }

    if(root->right != NULL)
    {
        rightLinkedList= convertBSTtoLinkedList(root->right); 
        newNode->next = rightLinkedList;
    }

    return head;
    }

valgrind the problem is somewhere in the while loop and i am assuming it is because i havent initialized that conductor->next is NULL, but i dont know how to do that since it it checking if it is NULL

  • If there's some awful `typedef ListNode * ListNodePtr;` and its like present in your program, [I suggest you get rid of them ASAP](https://stackoverflow.com/questions/3781932/is-typedefing-a-pointer-type-considered-bad-practice). – StoryTeller - Unslander Monica Nov 16 '17 at 16:16
  • 1
    If you follow the suggestions of valgrind to add in "--track-origins=yes" to your command line, it'll tell you where the uninitialised value comes from – Chris Turner Nov 16 '17 at 16:33
  • There's an unrelated bug too (what happens if `malloc()` runs out of memory and returns `NULL`?). – Brendan Nov 16 '17 at 16:47

2 Answers2

3

The problem is here:

newNode = malloc(sizeof(struct ListNode));
newNode->key = root->key;
head = newNode;

You create a new node and set the key, but you don't set the next pointer. While the value does get set if there is a right subtree, it will not get set if the node is a leaf node.

When your recursive call for the left side returns after processing a leaf node, it then checks the next pointer which was never written to. By reading an uninitialized value, you invoke undefined behavior.

Make sure that you initialize this pointer to NULL when you create the node:

newNode = malloc(sizeof(struct ListNode));
newNode->key = root->key;
newNode->next = NULL;
head = newNode;
dbush
  • 205,898
  • 23
  • 218
  • 273
0

Here what you can do is when you create a node in some place in your code initialize next with a NULL. That will most likely close this message of valgrind.

So add this line in your code

newNode->next = NULL;

The problem arises because the value that you are checking to be NULL is not initialized with NULL. As a result this checking is as if it is with some uninitialized value.

The solution I mentioned is just intializing every node with a proper next and key value.


Few points:

Check if the return value of malloc whether it failed or succeeded.

if ( newNode == NULL)
{
   fprintf(stderr,"Error");
   exit(1);
}
user2736738
  • 30,591
  • 5
  • 42
  • 56