Not sure where there is an unitialized value created by heap allocation

Question

Everything seems to be working fine, memory is allocating and freeing and doing what its supposed to be doing but when I check it with valgrind --track-origins=yes I get this conditional jump after entering in a name and a number.

==25590== Conditional jump or move depends on uninitialised value(s)
==25590==    at 0x4007BD: add_car (in /students/5/gmi6y5/cs2050/lab4/a.out)
==25590==    by 0x400704: main (in /students/5/gmi6y5/cs2050/lab4/a.out)
==25590==  Uninitialised value was created by a heap allocation
==25590==    at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==25590==    by 0x4006D9: main (in /students/5/gmi6y5/cs2050/lab4/a.out)

typedef struct FreightCars_
{
    char *name;
    int number;
    struct FreightCars_ *next_car;
}FreightCar;


int main(int argc, char *argv[])
{
    if(argc != 2)
    {
            printf("Insufficient number of arguements\n");
            return 0;
    }

    int size = atoi(argv[1]);
    FreightCar *engine = (FreightCar*)malloc(sizeof(FreightCar));
    int i;
    for(i=0;i<size;i++)
    {
            printf("Enter in freight car name and number: ");
            add_car(engine);
    }
    free_cars(engine);

 return 0;
}

void add_car(FreightCar *engine)
{
    FreightCar *newPtr = (FreightCar*)malloc(sizeof(FreightCar));
    newPtr->name = malloc(sizeof(char) * MAX_STR_LEN);

    if(newPtr == NULL)   
    {
            printf("Unable to allocate memory\n");
            exit(1);
    }
    scanf("%s", newPtr->name);
    scanf("%d", &newPtr->number);
    newPtr->next_car = NULL;

    if(engine->next_car == NULL)
    {
            engine->next_car = newPtr;
            printf("added at the beginning\n");     
    }
    else
    {
            FreightCar *currentPtr = engine;
            while(currentPtr->next_car != NULL)
            {
                    currentPtr = currentPtr->next_car;
            }
            currentPtr->next_car = newPtr;
            printf("added later\n");
    }
    free(newPtr->name);
}

void free_cars(FreightCar *engine)
{
    if(engine == NULL)
    {
            printf("Linked list is empty now\n");
    }
    else
    {
            free_cars(engine->next_car);
    }
    free(engine);
    engine = NULL;
}

You allocate memory, assign it to `engine`, but you don't initialise its contents anywhere. — Oliver Charlesworth, Jun 21 '14 at 22:55
I changed it to this and I still get same thing. `FreightCar *engine = NULL; engine = (FreightCar*)malloc(sizeof(FreightCar));` — user2930353, Jun 21 '14 at 23:05
Side note: the recursion depth of the `free_cars` function is equal to the length of the list. You can write that function with no recursion simply by storing the `next_car` pointer in a temporary variable, and then freeing the current item. — user3386109, Jun 21 '14 at 23:14
If you compile your program with the `-ggdb` flag, `valgrind` will be able to point out the individual lines on which events are taking place, rather than just the function name. Highly recommended. — , Jun 22 '14 at 00:03
consider using `FreightCar *engine = malloc( sizeof *engine );` . Less code-duplication and [less likely to hide an error](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — M.M, Jun 22 '14 at 03:14
Also there is a potential buffer overflow with `scanf("%s", newPtr->name);`. This is just as bad as using `gets`. You need to specify a length parameter with `scanf`, or use a different function such as `fgets`. Also perhaps allocate `MAX_STR_LEN+1` bytes, to allow for the null terminator. — M.M, Jun 22 '14 at 03:15

score 1 · Answer 1 · answered Jun 21 '14 at 23:10

In main you do

FreightCar *engine = (FreightCar*)malloc(sizeof(FreightCar));

then in a for loop you call

        add_car(engine);

add_car does

if(engine->next_car == NULL)

But as @Oli Charlesworth pointed out in a comment, you have not initialised the memory pointed to by engine, so you are making a decision based on unitialised memory contents here, hence the Valgrind complaint.

Not sure where there is an unitialized value created by heap allocation

1 Answers1