Sequence Stack: uninitialized value was created by a heap allocation

Question

Just now I wrote a simple data structure Sequence Stack in c, and met a problem.

==8142== Use of uninitialised value of size 4
==8142==    at 0x408F4AB: _itoa_word (_itoa.c:195)
==8142==    by 0x40935EA: vfprintf (vfprintf.c:1629)
==8142==    by 0x4099EFE: printf (printf.c:35)
==8142==    by 0x40664D2: (below main) (libc-start.c:226)
==8142==  Uninitialised value was created by a heap allocation
==8142==    at 0x402BB7A: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==8142==    by 0x80484C9: sqstack_create (sqstack.c:24)
==8142==    by 0x80485D8: main (sqstack.c:84)
==8142== 
==8142== Conditional jump or move depends on uninitialised value(s)
==8142==    at 0x408F4B3: _itoa_word (_itoa.c:195)
==8142==    by 0x40935EA: vfprintf (vfprintf.c:1629)
==8142==    by 0x4099EFE: printf (printf.c:35)
==8142==    by 0x40664D2: (below main) (libc-start.c:226)
==8142==  Uninitialised value was created by a heap allocation
==8142==    at 0x402BB7A: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==8142==    by 0x80484C9: sqstack_create (sqstack.c:24)
==8142==    by 0x80485D8: main (sqstack.c:84)

The results show that bug might occur in line 24:

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

#include "status.h"

#define MAX_SIZE 10

typedef int ElemType;

typedef struct{
    ElemType *base;
    ElemType *top;
    int      size;
} sqstack_t;


sqstack_t *sqstack_create(){

    sqstack_t *s = (sqstack_t *)malloc(sizeof(sqstack_t));
    if (s == NULL) {
        return ERROR;
    }

    //this line  24
    s->base = (ElemType *)malloc(sizeof(ElemType)*MAX_SIZE);
    if (s->base == NULL) {
        return ERROR;
    }

    s->top = s->base;
    s->size = 0;

    return s;
}

Status sqstack_push(sqstack_t *s, ElemType data){

    if (s->size > MAX_SIZE) {
        printf("This stack is full!\n");
        return ERROR;
    }

    *s->top++ = data;
    s->size += 1;

    return OK;
}


ElemType sqstack_pop(sqstack_t *s){

    if (s->size == 0) {
        printf("This stack is empty!\n");
    return ERROR;
    }

    ElemType data;

    data = *s->top--;
    s->size -= 1;

    return data;
}


ElemType sqstack_top(sqstack_t *s){

    return *(s->top);
}


void sqstack_destroy(sqstack_t *s){

    s->top = NULL;
    free(s->base);
    free(s);
}

int main(void){

    sqstack_t *s;

    s = sqstack_create();

    sqstack_push(s, 10);
    sqstack_push(s, 20);
    sqstack_push(s, 30);
    sqstack_push(s, 40);

    printf("%d\n", sqstack_pop(s));
    printf("-%d-\n", sqstack_top(s));

    sqstack_destroy(s);

    return 0;
}

I don't know how to fix it. I'm new to C and often confused with memory allocation and memory leaks. Could you recommend me any good materials or books?

Thanks!

Please [don't cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169). — unwind, May 23 '13 at 11:04
Your code is not showing the actual problem, which seems to be due to a call to `printf()` somewhere? — unwind, May 23 '13 at 11:05
Thanks to your suggestion on pointer cast! Because some books like Pointers on C do that, I just learn to do it. From now on, I correct bad code style. — oh1rouny, May 23 '13 at 14:56

score 1 · Accepted Answer · answered May 23 '13 at 11:03

1

With this line s->base = (ElemType *)malloc(sizeof(ElemType)*MAX_SIZE); you are allocating memory but its not initialized and have random values. So accessing s->base->somefield will result in random value.

Better to initialize it as

memset(s->base, 0, sizeof(*(s->base)));

or use calloc

s->base = (ElemType *)calloc(sizeof(ElemType)*MAX_SIZE, 1);

answered May 23 '13 at 11:03

Rohan

52,392
12
90
87

In particular, the line `s->top = s->base;` a few lines down from the `malloc()` copies the uninitialized value of `s->base` into `s->top`. Perhaps that line should have been `s->top = s->base = NULL;` – twalberg May 23 '13 at 14:49
`calloc()` works! Previously I only know `malloc()` to allocate memory and haven't realized that allocated memory should be initialized. Now I've got it. Thanks! – oh1rouny May 23 '13 at 15:01

score 0 · Answer 2 · edited May 23 '13 at 12:07

0

s->base is pointer to an array of size MAX_SIZE of ElemType elements. These elemType are not intialised. Perform memset on this pointer up to MAX_SIZE times.

edited May 23 '13 at 12:07

Regexident

29,441
10
93
100

answered May 23 '13 at 11:08

Dayal rai

6,548
22
29

Sequence Stack: uninitialized value was created by a heap allocation

2 Answers2