1

I'm trying to allocate memory for and initialize members of a struct.

In a function which takes a pointer to the struct and some other parameters (used in other members), I allocate the memory for the struct itself and its nodes member and initialize the other members. Printing the size and len members within the initialization function outputs the correct values, but testing them after the function outputs random garbage.

Why is this behavior occurring and what can I do to fix it?

Struct definitions:

struct node_t {
    int priority;
    void *data;
};

struct pqueue {
    int len,size,chunk_size;
    struct node_t *nodes;
};

Initialization function:

int alloc_pq(struct pqueue *q,int init_size,int chunk_size){
    // allocate for struct
    if((q=(struct pqueue*) malloc(sizeof(struct pqueue)))==NULL){
        perror("malloc");
        return -1;
    }

    // set initial sizes
    q->len=0;
    q->chunk_size=chunk_size;
    q->size=init_size;

    if(init_size>0){
        // allocate initial node memory (tried malloc here too)
        if((q->nodes=(struct node_t*) realloc(q->nodes,init_size*sizeof(struct node_t)))==NULL){
            perror("realloc");
            return -1;
        }
    }

    printf("%lu %d %d\n",sizeof(*q),q->size,q->len); // DEBUG
    return q->size;
}

In main function:

struct pqueue q;

...

alloc_pq(&q,n,0);
printf("%lu %d %d\n",sizeof(q),q.size,q.len); // DEBUG

Output (second last number is always > 32000, last is seemingly random):

24 67 0
24 32710 -2085759307
Valkyrie
  • 841
  • 1
  • 9
  • 22
  • [should I cast the result of malloc?](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Barmar Feb 08 '18 at 16:30
  • `realloc` is definitely wrong when allocating memory for the nodes member. `malloc` or `calloc` is correct. – bruceg Feb 08 '18 at 16:32
  • You should use `malloc`, not `realloc`. You can only use `realloc` when the first argument is a pointer that was previously returned by `malloc/realloc`, or it's `NULL`. Neither of these is true in your function, you're passing an uninitialized pointer. – Barmar Feb 08 '18 at 16:32
  • also the value of `q` won't change after return, since it is passed by value. You need to declare your function: `int alloc_pq(struct pqueue **q,int init_size,int chunk_size)` – bruceg Feb 08 '18 at 16:34
  • @Barmar as I mentioned in the commenting I tried malloc and it made no difference to the issue. I thought I might as well try it and see what happens. – Valkyrie Feb 08 '18 at 16:34

1 Answers1

1

The way you do things is making no changes. You passed address and then overwrote it.

Any changes you make to q will be lost when that function ends. Solution would be to either take a pointer variable in main() and pass it's address.

struct pqueue* q;

...

alloc_pq(&q,n,0);

Change alloc_pq accordingly. Something like

int alloc_pq(struct pqueue **q,int init_size,int chunk_size){
    // allocate for struct
    if((*q=malloc(sizeof(struct pqueue)))==NULL){
        perror("malloc");
        return -1;
    }

    // set initial sizes
    (*q)->len=0;
    (*q)->chunk_size=chunk_size;
    (*q)->size=init_size;

    if(init_size>0){
        // allocate initial node memory (tried malloc here too)
        if(((*q)->nodes= realloc((*q)->nodes,init_size*sizeof(struct node_t)))==NULL){
            perror("realloc");
            return -1;
        }
    }

    printf("%lu %d %d\n",sizeof(*q),(*q)->size,(*q)->len); // DEBUG
    return (*q)->size;
}

Your use of realloc is wrong. Use a temporary vcariable to hold the result and check whether it returns NULL or not.

Casting the result of malloc,realloc is not needed. Free the memory allocated when you are done working with it. Check the return value of malloc,realloc.

struct node_t* temp= realloc((*q)->nodes,init_size*sizeof(struct node_t)));
if( temp == NULL ){
   perror("realloc");
   exit(EXIT_FAILURE);
}
(*q)->nodes = temp;
user2736738
  • 30,591
  • 5
  • 42
  • 56
  • Marked as accepted, but I'm casting `*alloc` family calls for compatibility with C++ compilers, as I may need that later. – Valkyrie Feb 08 '18 at 16:45