3

i'm new to C and i'm stucked with a common problem but really i'm not able to understand the reason in my case.

i have this method whose purpose is to initialize a Map with a specified name and check if it exists in a RED-BLACK Tree.

void reduceMap(char *mapName){

    Map *tempMap = (Map *) malloc(sizeof(Map));
    struct rbNode *tempNode = (struct rbNode *) malloc(sizeof(struct rbNode));

    copyName = stringCopy(mapName);

    tempMap->name = copyName;
    tempNode->data = tempMap;

    ...DO SEARCH HERE...

    freeMap(tempNode);

}

So i allocate memory for a Node which contains a Map, do the research and in the end free the memory of the temporary objects created. The structures of my objects are the following:

typedef struct{
    char *name;
    char *specification;
    Point *start, *end;
}Map;

struct rbNode{
    Color color;
    void *data;
    struct rbNode *parent, *leftChild, *rightChild;
};

The method stringCopy and freeMap are:

void freeMap(struct rbNode *node){
    if(node){

         free( ((Map *)node->data)->name );

         if(((Map *)node->data)->specification != NULL) 
             free( ((Map *)node->data)->specification );        

         free( (Map *)node->data );
         free(node);
     }
 }


 char *stringCopy(const char *source){
     char *copy = (char *)malloc(strlen(source) + 1);

     strcpy(copy,source);
     return copy;
 }

I'm testing the program using valgrind and this is the error i get:

Conditional jump or move depends on uninitialised value(s)
==5215==    at 0x8049DF3: freeMap (in /home/ve/Dropbox/progetto2016/Percorsi/percorsi)
==5215==    by 0x8048C40: reduceMap (in /home/ve/Dropbox/progetto2016/Percorsi/percorsi)
==5215==    by 0x80489DA: main (in /home/ve/Dropbox/progetto2016/Percorsi/percorsi)
==5215==  Uninitialised value was created by a heap allocation
==5215==    at 0x402C17C: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==5215==    by 0x8048B0A: reduceMap (in /home/ve/Dropbox/progetto2016/Percorsi/percorsi)
==5215==    by 0x80489DA: main (in /home/ve/Dropbox/progetto2016/Percorsi/percorsi)

I understand that the problem is caused by the allocation of tempMap and it seems that there is something not initialized int it but why? I only care about the name and the specification... I really do not understand.

Any help would be appreciate. Thank you

VeVeVez
  • 90
  • 9

2 Answers2

2

Simply put, it seems that in your reduceMap, this happens

Map *tempMap = (Map *) malloc(sizeof(Map));
tempMap->name = copyName;
tempMap->specification = /* what!?? */;

You don't initialize the entire allocated struct. Consider writing a map constructor (in the form of an initMap function), to match your destructor (freeMap).

Side note : Don't cast the result of malloc.

Community
  • 1
  • 1
StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • Thank you @StoryTeller also for the note. I thought that the default value of char *specification in the struct was automatically NULL and evaluating (Map *)node->data)->specification with NULL was not a problem – VeVeVez Sep 04 '16 at 14:07
  • 1
    @VeVeVez, it isn't a requirement. The value there is, simply put, unspecified. And relying on it being anything, is relying on undefined behavior. Sadly, many debuggers may zero initialize memory under certain build configuration, making those bugs hard to track. – StoryTeller - Unslander Monica Sep 04 '16 at 14:46
1

You do not initialize specification member in map

typedef struct{
    char *name;
    char *specification;
    Point *start, *end;
} Map;

but later you use this unintialized value to conditionally jump (if statement):

if(((Map *)node->data)->specification != NULL)

SOLUTION

Initialize all members of Map before you use them.

4pie0
  • 29,204
  • 9
  • 82
  • 118
  • Thank you @where_is_tftp, so if i understand the point is that i'll have to initialize EVERY field that i'm planning to use/evaluate in my free function. I thought that the default value of `char *specification` in the struct was automatically NULL and evaluating `(Map *)node->data)->specification` with NULL was not a problem... my fault – VeVeVez Sep 04 '16 at 14:05
  • 1
    @VeVeVez It is undefined behaviour in C to use uninitialized variable. Remember this rule. In C variables allocated on heap are uninitialized as well as local variables. You must initialize them before use. – 4pie0 Sep 04 '16 at 14:37