How to fix 'Use of uninitialised value' and how to find location of leaks?

Question

I wrote a small c programm which gets 3 names as an input and then stores this three values to a node. The three values (names) are stored in an array JOHNJAKEMATT, each name has 4 characters. The resulted Node looks like this:

/*
pointer_to_linked_list
       |
       v
    node[2]          node[1]              node[0]
       |                |                    |
       |                |                    |
+------+---+     +------+---+        +-------+------+ 
| MATT | o-----> | JAKE | o--------> |  JOHN | NULL | 
+------+---+     +------+---+        +-------+------+ 
*/

All good, everything runs as expected. Each name is stored temporary in an array, then moved to a data in a node. To achieve this, I allocated memory and if there comes a failure I wrote a function which should free all memory if memory allocation is failing:

void freeNode(struct _Node_ *current_node)
{
struct _Node_ *tmp_node;
while(current_node != NULL) {
    tmp_node = current_node;
    current_node = current_node->next;
    free(tmp_node);
}
return;
}

Also I wrote a function, that prints out my linked-list:

void printNodes(struct _Node_ *current_node) {
    while(current_node != NULL) {
        printf("%s\n", current_node->data);
        current_node = current_node->next;
    }
}

The result in the console is the linked-list with each value:

MATT
JAKE
JOHN

Everything works, then I wanted to check with valgrind (valgrind --leak-check=yes --track-origins=yes ./main) if there are any errors - and yes there are 2 types of errors:

• uninitalised values, I really don't know where they could be..
• 3 memory leaks, has anyone a clue where they could be?

Full valgrind report:

==20043== Memcheck, a memory error detector
==20043== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==20043== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==20043== Command: ./main
==20043== 
MATT
==20043== Conditional jump or move depends on uninitialised value(s)
==20043==    at 0x1094BC: printNodes (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10928F: main (in /home/osboxes/Dropbox/ESP/main)
==20043==  Uninitialised value was created by a stack allocation
==20043==    at 0x10935C: saveToLinkedList (in /home/osboxes/Dropbox/ESP/main)
==20043== 
==20043== Use of uninitialised value of size 8
==20043==    at 0x1094A0: printNodes (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10928F: main (in /home/osboxes/Dropbox/ESP/main)
==20043==  Uninitialised value was created by a stack allocation
==20043==    at 0x10935C: saveToLinkedList (in /home/osboxes/Dropbox/ESP/main)
==20043== 
JAKE
==20043== Use of uninitialised value of size 8
==20043==    at 0x1094AF: printNodes (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10928F: main (in /home/osboxes/Dropbox/ESP/main)
==20043==  Uninitialised value was created by a stack allocation
==20043==    at 0x10935C: saveToLinkedList (in /home/osboxes/Dropbox/ESP/main)
==20043== 
JOHN
==20043== Conditional jump or move depends on uninitialised value(s)
==20043==    at 0x1094F4: freeNode (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10929B: main (in /home/osboxes/Dropbox/ESP/main)
==20043==  Uninitialised value was created by a stack allocation
==20043==    at 0x10935C: saveToLinkedList (in /home/osboxes/Dropbox/ESP/main)
==20043== 
==20043== Use of uninitialised value of size 8
==20043==    at 0x1094DB: freeNode (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10929B: main (in /home/osboxes/Dropbox/ESP/main)
==20043==  Uninitialised value was created by a stack allocation
==20043==    at 0x10935C: saveToLinkedList (in /home/osboxes/Dropbox/ESP/main)
==20043== 
==20043== Conditional jump or move depends on uninitialised value(s)
==20043==    at 0x4838931: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20043==    by 0x1094EE: freeNode (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10929B: main (in /home/osboxes/Dropbox/ESP/main)
==20043==  Uninitialised value was created by a stack allocation
==20043==    at 0x10935C: saveToLinkedList (in /home/osboxes/Dropbox/ESP/main)
==20043== 
==20043== 
==20043== HEAP SUMMARY:
==20043==     in use at exit: 15 bytes in 3 blocks
==20043==   total heap usage: 7 allocs, 4 frees, 1,087 bytes allocated
==20043== 
==20043== 5 bytes in 1 blocks are definitely lost in loss record 1 of 2
==20043==    at 0x483774F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20043==    by 0x1091D4: copyToNewNode (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10942E: saveToLinkedList (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10932F: readInput (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x109271: main (in /home/osboxes/Dropbox/ESP/main)
==20043== 
==20043== 10 bytes in 2 blocks are definitely lost in loss record 2 of 2
==20043==    at 0x483774F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20043==    by 0x1091D4: copyToNewNode (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x109459: saveToLinkedList (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x10932F: readInput (in /home/osboxes/Dropbox/ESP/main)
==20043==    by 0x109271: main (in /home/osboxes/Dropbox/ESP/main)
==20043== 
==20043== LEAK SUMMARY:
==20043==    definitely lost: 15 bytes in 3 blocks
==20043==    indirectly lost: 0 bytes in 0 blocks
==20043==      possibly lost: 0 bytes in 0 blocks
==20043==    still reachable: 0 bytes in 0 blocks
==20043==         suppressed: 0 bytes in 0 blocks
==20043== 
==20043== For counts of detected and suppressed errors, rerun with: -v
==20043== ERROR SUMMARY: 14 errors from 8 contexts (suppressed: 0 from 0)

Full Code of main.c:

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

struct _Node_ 
{
    char *data;
    struct _Node_ *next;
};

struct _Node_ *copyToNewNode(char *pointer_value_linked_list, struct _Node_ *nextnode);
struct _Node_ *readInput(char *pointer_buffer_config, int len_char);
struct _Node_ *saveToLinkedList(char *pointer_tmp_buffer, int str_len, int pos_node);
void printNodes(struct _Node_ *current_node);
void freeNode(struct _Node_ *current_node);

struct _Node_ *copyToNewNode(char *pointer_value_linked_list, struct _Node_ *nextnode) 
{
    struct _Node_ *new_node_pointer;    
    new_node_pointer = malloc(sizeof(struct _Node_));
    if (!new_node_pointer) 
    { 
        freeNode(new_node_pointer);
        return NULL; 
    }

    new_node_pointer->data = malloc(strlen(pointer_value_linked_list) + 1);
    if (!new_node_pointer->data) 
    {
        freeNode(new_node_pointer);
        return NULL;
    }

    strcpy(new_node_pointer->data,pointer_value_linked_list); 
    new_node_pointer->next = nextnode;

    return(new_node_pointer);
}

int main()
{
    char buffer_config[] = "JOHNJAKEMATT";
    struct _Node_ *pointer_to_linked_list;

    pointer_to_linked_list = readInput(buffer_config, strlen(buffer_config));
    if(pointer_to_linked_list == NULL)
    {
        //nodes have already been freed
        return 0;
    }

    printNodes(pointer_to_linked_list);
    freeNode(pointer_to_linked_list);

    return 1;
}

struct _Node_ *readInput(char *pointer_buffer_config, int len_char)
{
    char tmp_buffer_input[5];
    int counter;
    struct _Node_ *pointer_last_node; // points to last successful saved node
    int pos_node = 0;
    int counter_next_node = 0;

    while(counter_next_node != 3)
    {
        for(counter = 0; counter < 4; counter++)
        {
            tmp_buffer_input[counter] = *pointer_buffer_config;
            pointer_buffer_config++; 
        }
        tmp_buffer_input[4] = '\0';

        pointer_last_node = saveToLinkedList(tmp_buffer_input, strlen(tmp_buffer_input), pos_node);

        ++pos_node;
        counter_next_node++;
    }
    return pointer_last_node;
}

struct _Node_ *saveToLinkedList(char *pointer_tmp_buffer, int str_len, int pos_node)
{
char value_linked_list[str_len+1];
int counter = 0;

while(counter != str_len)
{
    value_linked_list[counter] = *(pointer_tmp_buffer+counter);
    counter++;
}
value_linked_list[counter] = '\0';

struct _Node_* node[3];
if(pos_node == 0)  
{
    node[pos_node] = copyToNewNode(value_linked_list, NULL);
}
else
{
    node[pos_node] = copyToNewNode(value_linked_list, node[pos_node-1]);
}  
return node[pos_node];
}

void printNodes(struct _Node_ *current_node) {
    while(current_node != NULL) {
        printf("%s\n", current_node->data);
        current_node = current_node->next;
    }
}

void freeNode(struct _Node_ *current_node)
{
    struct _Node_ *tmp_node;
    while(current_node != NULL) {
        tmp_node = current_node;
        current_node = current_node->next;
        free(tmp_node);
    }
    return;
}

I would be extremely grateful if someone could help me locating this errors, even after reading the valgrind manual I don't know where the errors could be. Thanks.

Answer 1

There is another bug not spotted in 2018. This copying loop:

while(counter != str_len)

fails to copy the null terminating character. The strcpy function should just have been used. Another way to fix it is while (counter <= str_len), or to zero-initialize the destination array.

This is a possible source of an uninitialized stack byte being identified, because the copying is going into a VLA, which is then duplicated into dynamic storage in the linked list.

Answer 2

into saveToLinkedList

struct _Node_* node[3];
if(pos_node == 0)  
{
  node[pos_node] = copyToNewNode(value_linked_list, NULL);
}
else
{
  node[pos_node] = copyToNewNode(value_linked_list, node[pos_node-1]);
}  

struct _Node_* node[3] is not initialized, when pos_node is not 0 you will use an entry of the vector not initialized => copyToNewNode will create a node with a random next pointer

struct _Node_* node[3]; must be static struct _Node_* node[3];

In freeNode() you missed to free the field data, must be like :

void freeNode(struct _Node_ *current_node)
{
   struct _Node_ *tmp_node;
   while(current_node != NULL) {
      tmp_node = current_node;
      current_node = current_node->next;
      if (tmp_node->data != NULL)
         free(tmp_node->data);
      free(tmp_node);
   }
}