Valgrind / Memory Errors

Question

I'm receiving the following errors repeatedly throughout my code when I use valgrind. I'm not quite sure what these mean and I can't identify the uninitialized values.

==16795== Conditional jump or move depends on uninitialised value(s)
==16795==    at 0x4A06E8A: strcmp (mc_replace_strmem.c:412)
==16795==    by 0x4009C7: dictionary_add (libdictionary.c:44)
==16795==    by 0x40061B: main (part2.c:28)
==16795== 
==16795== Invalid write of size 1
==16795==    at 0x4A082E7: strcpy (mc_replace_strmem.c:303)
==16795==    by 0x400AA8: dictionary_add (libdictionary.c:57)
==16795==    by 0x40061B: main (part2.c:28)
==16795==  Address 0x4c361a3 is 0 bytes after a block of size 3 alloc'd
==16795==    at 0x4A05E1C: malloc (vg_replace_malloc.c:195)
==16795==    by 0x400931: node_newnode (libdictionary.c:28)
==16795==    by 0x400A8C: dictionary_add (libdictionary.c:54)
==16795==    by 0x40061B: main (part2.c:28)

I'm creating a linked-list data structure and these are the functions being called when this memory error is given.

// Here are the constructions of both a node and the dictionary data struct.

typedef struct node
{
char* key;
char* value;
struct node* next;
}node;


typedef struct _dictionary_t
{
node* head;

} dictionary_t;

// First a new dictionary is created

void dictionary_init(dictionary_t *d)
{
d->head = NULL;
d->head = malloc(sizeof(node));
node_init(d->head);
}

// A new node is created using the node_init method.

void node_init(node *n)
{
n->key = NULL;
n->value =NULL;

n->key = malloc(sizeof(char));
n->value = malloc(sizeof(char));
n->next = NULL;
}

// This new node method is used after the original initialization (when we are adding new terms) The previous one is specifically for creating a sentinel at the beginning of the structure upon creation.

void node_newnode(node *n, int x, int y)
{
n->key = NULL;
n->value = NULL;

n->key = malloc(x*sizeof(char));
n->value = malloc(y*sizeof(char));
n->next = NULL;
}

// This function is also being called to add "key" and "value" as a pair in this case.

int dictionary_add(dictionary_t *d, const char *key, const char *value)
{
node *current;
current =  d->head;
if(strcmp(current->key,key)==0)
        return -1;
while(current->next != NULL){
        current=current->next;
        if(strcmp(current->key,key)==0)
                return -1;
}
        current->next = NULL;
        current->next = malloc(sizeof(node));

        node_newnode(current->next,strlen(key),strlen(value));

        current = current->next;
        strcpy((current->key), key);
        strcpy((current->value),value);
        return 0;
}

Anyone have any ideas on why I'm getting those errors. The main method has thus far only created a dictionary and called the add function. Valgrind is reporting 45+ errors over 38 contexts over the course of 8 function calls. I think these are probably small errors that I'm committing repeatedly.

I'm now also getting the same errors with this function. A valgrind --track-origins=yes command traces it to a stack allocation in the following function:

int dictionary_parse(dictionary_t *d, char *key_value)
{
char* colon;
char* space;
colon = key_value;
space = key_value;

space++;

int key_length = -1; //Default key length to check for failure

int i=0;
int j=0;   // Loop variables
int k=0;

int length = strlen(key_value);

for(i=0;i<length-2;i++){
        if(*colon == ':' && *space == ' '){
                key_length = i;
                break;
        }
        colon++;
        space++;
}

if(key_length == -1 || key_length == 0)
        return -1;

int value_length = length-2-key_length;

colon = key_value;


char key_word[key_length];
key_word[0] = '\0';
char value_word[value_length];
value_word[0] = '\0';

for(j=0;j<key_length;j++){
key_word[j] = *colon;
colon++;
}

space++;

for(k=0; k<value_length;k++){
value_word[k] = *space;
space++;
}


char* finalkey[key_length];
strcpy((char*)finalkey,key_word);
char* finalvalue[value_length];
strcpy((char*)finalvalue,value_word);

dictionary_add(d,(char*)finalkey,(char*)finalvalue);    

return 0;

}

Thanks, -newprogrammer

Answer 1

The first error ("Conditional jump or move depends on uninitialised value(s)") is because the character pointed to by current->key was never initialised. You allocated a byte for it in node_init(), but you never actually set that byte to anything. If you want the head node's key to act like an empty string, change node_init() like this:

n->key = malloc(sizeof(char));
n->key[0] = '\0';

The second error ("Invalid write of size 1") is because your node_newnode() function is allocating one byte less than is needed for the strings. strlen(key) counts the actual characters in the string, but one more byte is needed for the null terminator character. Use:

node_newnode(current->next,strlen(key) + 1,strlen(value) + 1);

(but personally I would just pass key and value to node_newnode(), and have it do both the allocation and the strcpy()).