CODEWITHSUNDEEP
clinuxstructmallocsystem
alvinzoo
alvinzoo

Reputation: 513

Weird behavior of malloc

I am trying to dynamically initializing a queue. Here is my function.

typedef struct{
    int size;
    int max_size;
    short * eles;
} queue;

void dump_queue(queue *q)
{
    //print a bunch of information
}

void init_queue(queue *q, int max_size)
{
    q = (queue)malloc(sizeof(queue));
    q->size = 0;
    q->max_size = max_size;
    q->eles = (short *)malloc(max_size * sizeof(short));
    int i;
    for(i = 0; i < max_size; i++)
        q->eles[i] = -1;
    dump_queue(q);
}

task_queue is a global variable. The structure of the routine is like:(not exact code)

//globally defined here but not initialized 
queue * task_queue;
void init_scheduler()
{
    init_queue(task_queue, 32);

    dump_queue(task_queue);
    //other staff
}

note that there are two dump_queue, one is init_queue(), the other is after init_queue. Since task_queue is malloced, I expect that two calls of dump_queue should give a same result. But the second one actually reported a SIG_FAULT.

After I checked, in the second call of dump_queue, task_queue is actually NULL. Why is that?

Upvotes: 1

Views: 165

Answers (4)

Charlon
Charlon

Reputation: 363

if you use this function only to initialize task_queue you should do that

void init_global_queue(int max_size)
{
    queue *task_queue = malloc(sizeof(queue));
    task_queue->size = 0;
    task_queue->max_size = max_size;
    task_queue->eles = malloc(max_size * sizeof(short));
    int i;
    for(i = 0; i < max_size; i++)
        task_queue->eles[i] = -1;
}

But unfortunately, this is not a good pattern because you can't use this function more than once

I think the best way of doing it is like this

queue *init_queue(int max_size)
{
    queue *q = malloc(sizeof(queue));
    q->size = 0;
    q->max_size = max_size;
    q->eles = malloc(max_size * sizeof(short));
    int i;
    for(i = 0; i < max_size; i++)
        q->eles[i] = -1;
    return q;
}

and when you want to initialize any queue you'll just have to call this function

queue * task_queue;
void init_scheduler()
{
    task_queue = init_queue(32);

    dump_queue(task_queue);
}

And 2 things :

-Casting the return value of a malloc call is not really bad, in fact it's casted anyway, explicitly or implicitly

-The best way of doing a malloc and (for this example) returning its value is like this :

type *my_pointer = NULL;
my_pointer = malloc(sizeof (type))
if (my_pointer == NULL)
    return NULL;
my_pointer->foo = 0;
my_pointer->bar = NULL;
...
return my_pointer

So that if malloc doesn't work properly(i.e. for example : if your RAM is full, malloc will return NULL (and set errno to ENOMEM)), you can exit your program properly

and if you like minimalist programming, you can do it like this to ;)

type *my_pointer = NULL;
if (!(my_pointer = malloc(sizeof (type))))
    return NULL;

more ? man malloc, man errno

Upvotes: 0

Natasha Dutta
Natasha Dutta

Reputation: 3272

Why not? you're allocating memory in the function local scope of init_queue().

The scope of the returned pointer is valid, but the assignment is not valid.

q = malloc(sizeof(queue));

this q won't hold it's value outside init_queue() function.

if queue * task_queue; is global, is it really required to pass it as function parameter?

Also, as a note, please do not cast the return value of malloc().

EDIT:

No, there is no auto-free() concept in c. It will result in a memory leak, if not free()-d by the application explicitly.

Upvotes: 5

JS1
JS1

Reputation: 4767

You never assign anything to task_queue. Notice that you passed task_queue to init_queue by value instead of by reference. You should either modify init_queue to return a queue *, or modify its first argument to take a queue ** and pass in &task_queue from init_scheduler.

Or perhaps the easiest fix, since it is a global, just do task_queue = q; at the end of init_queue.

Upvotes: 2

Peter Miehle
Peter Miehle

Reputation: 6080

it should either be init_queue(&task_queue); or task_queue=init_queue();

Upvotes: 1

Related Questions