segfault when allocate memory dynamically in C

I've been trying to build a priority queue in C.

First of all, I do some initialization work such as allocating space.

The following is the Initialize routine and PriorityQueue is a pointer.

void Initialize(int MaxElement, PriorityQueue H)
{
   if (MaxElement < MinPQSize)
     printf("Priority queue size is too small");

   if (!(H = (PriorityQueue)malloc(sizeof(struct HeapStruct))开发者_StackOverflow社区))
     printf("Out of space!!!");

   if (!(H->Elements = (ElementType *)malloc((MaxElement+1) * sizeof(ElementType))))
     printf("Out of space!!!");

   H->Capacity = MaxElement;
   H->Size = 0;

   H->Elements[0] = MinData;
}

Here is how the test code is like

 int MaxElement = 15;
 PriorityQueue myHeap;
 Initialize(MaxElement, myHeap);

But when I try to insert elements into the heap, a segmentation fault pops out.

It can be solved by simply returning the PriorityQueue pointer from Initialize routine.

 PriorityQueue Initialize(int MaxElement, PriorityQueue H)
 {
   ...
   return H;
 }
 myHeap = Initialize(MaxElement, myHeap);

So what's happening under the hood?

Is free() invoked when the function returns without a return value?

Thx in advance!

---

No, even though the H that you're passing in is a pointer, you're trying to change it within the function (with your first malloc). In order to change something, you need to pass a pointer to it. In this case, that means a pointer to a pointer:

void Initialize (int MaxElem, PriorityQueue *H) {
    if (MaxElem < MinPQSize)
        printf("Priority queue size is too small");

    if (!(*H = (PriorityQueue)malloc(sizeof(struct HeapStruct))))
        printf("Out of space!!!");

    if (!((*H)->Elements = (ElemType *)malloc((MaxElem+1) * sizeof(ElemType))))
        printf("Out of space!!!");

    (*H)->Capacity = MaxElem;
    (*H)->Size = 0;
    (*H)->Elements[0] = MinData;
}

Without the extra level on indirection, the H that you change within the function is isolated to the function - it is not reflected back to the caller.

A couple of other points you may want to consider:

• You shouldn't cast the return from malloc, it can hide certain errors that you really do want to know about.
• If your second malloc fails, you should free the result of the first malloc.
• If either of your malloc calls fail, you should return rather than continue, since continuing will cause undefined behaviour if you dereference the null pointer.
• You probably don't want to print things from general purpose functions since that's probably an unwanted behaviour. If you must indicate a problem, you're better off passing back an indication to the caller to let them handle it in their own way.

Although to be honest, I actually like the versions that return a value (with no need to pass it in beforehand since you're clearly creating a new thing). Something like this should do:

PriorityQueue Initialize (int MaxElem) {
    PriorityQueue H;

    if (MaxElem < MinPQSize) {
        printf("Priority queue size is too small");
        return NULL;
    }

    if (!(H = malloc(sizeof(*H)))) {
        printf("Out of space!!!");
        return NULL;
    }

    if (!(H->Elements = malloc((MaxElem+1) * sizeof(ElementType)))) {
        printf("Out of space!!!");
        free (H);
        return NULL;
    }

    H->Capacity = MaxElem;
    H->Size = 0;
    H->Elements[0] = MinData;

    return H;
}

PriorityQueue myHeap = Initialize (MaxElement);

---

You are passing the pointer by value, allow me to illustrate:

char* c = 0;

void set_c(char* ptr)
{
    ptr = (char*) malloc(sizeof(char) * 10);       
}

// a copy of c is sent in, 
set_c(c);
// c doesn't point to the newly allocated data!

To set it correctly, you have to pass your pointer BY pointer, like this:

void set_c_correctly(char** ptr)
{
   *ptr = (char*) malloc(sizeof(char) * 10);
}

// a pointer to c is passed in
set_c_correctly(&c);

// now c points to the newly allocated data