"Heap corruption detected" Error

Hi,

I have an assignment on pointers and chain structures.
Basically, we wrote a program that acts like flight ticketing system (add flight, add passenger, assign passenger to flight, delete flight from passenger data, etc.)

I wrote the code up to the part that I need to remove a flight from a passenger's flights record. I have un-linked the struct which I wanted to free (still keeping a temporary pointer pointing at its address) and when I try to free it, I get this HEAP error.

I read that it has got something to do with memory leak somewhere after the malloc() function but I really have no clue on how to debug this kind of problem.

I'll appreciate any advice...

Use valgrind.
(Or something like ElectricFence which I recently discovered).

more likely than a leak is that somewhere you are overwriting the allocated memory blocks in a way that corrupts the heap info that is part of the allocated block. check where you are writing to the allocated blocks to be sure you aren't exceeding some array bounds or string lengths or something.

Thanks for the suggestions...I dont know how to call it but what I ended up doing was to take the "free" function and place it in random places after the line where I used its "malloc".
Weird as it seems, the compiler does not allow me to point a pointer to NULL. If I do point something to NULL it trashes the pointer and gives all kinds of access violation errors and stuff.

Is there a problem with assigning NULL to a pointer?

Quote:

---

Originally Posted by tmrsgv

Thanks for the suggestions...I dont know how to call it but what I ended up doing was to take the "free" function and place it in random places after the line where I used its "malloc".
Weird as it seems, the compiler does not allow me to point a pointer to NULL. If I do point something to NULL it trashes the pointer and gives all kinds of access violation errors and stuff.

---

That does not sound like a solution, or even a way to really track down the bug(s). manasij7479 gave you some suggestions. You could learn to use your debugger, which should stop your program when it detects the heap corruption, and you can find out where in the code it happened, down to the exact line. You can then examine all the variables, including pointers and the data they point to, to track down your problem. Debuggers are really worth learning to use, they are are perhaps the most powerful tool a programmer has.

Quote:

---

Is there a problem with assigning NULL to a pointer?

---

No, no problem. Actually, NULL is there specifically to be assigned to pointers, so you can point them to invalid memory, or "nowhere". You can point any pointer there, but you shouldn't go trying to access the "memory" at NULL (technically there is no memory, since it's not a valid address). You also should make sure you don't go messing with the memory pointed to by uninitialized pointers.


My intuition says dmh2000 is correct, you have some sort of buffer overflow or something in memory you malloc. Perhaps if you actually posted the code causing the problems, we could actually help you, instead of this vague banter about pointers in C.

Quote:

---

Originally Posted by tmrsgv

Is there a problem with assigning NULL to a pointer?

---

No. Virtually every program in existence does it.

It would most likely mean that you are dereferencing the pointer after assigning NULL to it.

Quote:

---

Originally Posted by anduril462

Perhaps if you actually posted the code causing the problems, we could actually help you, instead of this vague banter about pointers in C.

---

What I'm posting are two functions: one for adding a passenger to a flight and the other is to cancel a flight ticket per passenger.

Code:

---

void buy_ticket(flights_list* flights, passengers_list* passengers, int number, int id)
{
    passengers_list* temp_passenger = (passengers_list*) malloc (sizeof (passengers));
    flights_list* temp_flight = (flights_list*) malloc (sizeof (flights));
    flights_list* temp_flight_head = flights;
    passengers_list* temp_flight_point;
    passengers_list* temp_pass_head = passengers;
    if (temp_flight == NULL || temp_passenger == NULL)
    {
        printf ("Error allocating memory");
        exit(1);
    }
    if (passengers != NULL && flights != NULL)
    {
        while (id != temp_pass_head ->info->id)
            temp_pass_head = temp_pass_head ->next;
        while (number != temp_flight_head ->info ->number)
            temp_flight_head = temp_flight_head ->next;
    }
    temp_flight ->info = temp_flight_head ->info;
    temp_flight ->next = NULL;
    temp_passenger ->info = NULL;
    temp_passenger ->next = NULL;
    temp_passenger ->info = temp_pass_head ->info;
    if (temp_flight_head ->info ->passengers != NULL)
        temp_flight_point = temp_flight_head ->info ->passengers;
    if (temp_flight_head ->info ->passengers == NULL)
        temp_flight_head ->info ->passengers = temp_passenger;
    else
    {
        if (temp_flight_head ->info ->passengers->info->id > temp_passenger ->info ->id)
        {
            temp_passenger ->next = temp_flight_head ->info ->passengers;
            temp_flight_head ->info ->passengers = temp_passenger;
        }
        if (temp_flight_point ->next != NULL && temp_flight_head ->info ->passengers->info->id < temp_passenger ->info ->id)
        {
            while ((temp_flight_point ->next != NULL) && (temp_flight_point ->next ->info->id < temp_passenger ->info ->id))
                temp_flight_point = temp_flight_point ->next;
            temp_passenger ->next = temp_flight_point ->next;
            temp_flight_point ->next = temp_passenger;
        }
        if (temp_flight_head ->info ->passengers->info->id < temp_passenger ->info ->id && temp_flight_head ->info ->passengers->next == NULL)
            temp_flight_head ->info ->passengers ->next = temp_passenger;
    }
   
    if (temp_passenger ->info ->flights == NULL)
    {
        temp_passenger ->info ->flights = temp_flight;
    }
    else
    {
        while (temp_passenger ->info ->flights ->next != NULL && temp_flight ->info ->number > temp_passenger ->info ->flights ->next ->info ->number)
            temp_passenger ->info ->flights = temp_passenger ->info ->flights ->next;
        if (temp_flight ->info ->number < temp_passenger ->info ->flights ->info ->number)
        {
            temp_flight ->next = temp_passenger ->info ->flights;
            temp_passenger ->info ->flights = temp_flight;
        }
        else
        {
            temp_flight ->next = temp_passenger ->info ->flights ->next;
            temp_passenger ->info ->flights ->next = temp_flight;
        }
    }
   
}


void cancel_ticket(flights_list* flights, int number, int id)
{
    flights_list* passenger_flights;
    passengers_list* passengers_on_flight;
    passenger* passenger;
    passengers_list* temp_list;
    while (flights ->info->number != number)
        flights = flights ->next;
    passengers_on_flight = flights ->info->passengers;
    if (passengers_on_flight ->info ->id != id)
    {
        while (passengers_on_flight ->next ->info ->id != id)
            passengers_on_flight = passengers_on_flight ->next;
        temp_list = passengers_on_flight ->next;
        passenger = passengers_on_flight ->next->info;
        passenger_flights = passengers_on_flight->next ->info ->flights;
        passengers_on_flight ->next = passengers_on_flight ->next ->next;
        free (temp_list);
    }
    else
    {
        passenger = passengers_on_flight ->info;
        passenger_flights = passengers_on_flight ->info ->flights;
        flights ->info ->passengers = flights ->info ->passengers ->next;
        free (passengers_on_flight);
    }
    if (passenger_flights ->info ->number != number)
    {
        while (passenger_flights ->next ->info ->number != number)
            passenger_flights = passenger_flights ->next;
        passenger_flights ->next = passenger_flights ->next ->next;
        free (passenger_flights);
    }
    else
    {
        passenger ->flights = passenger->flights->next;
        free (passenger ->flights);
    }
   
}

---

Is it just me or did anyone else not get past the first two lines either?

Code:

---

passengers_list* temp_passenger = (passengers_list*) malloc (sizeof (passengers));
flights_list* temp_flight = (flights_list*) malloc (sizeof (flights));

---

Edit: And what's with the spaces in between the pointer arrows?

First of all, I just started C two months ago, so I wouldn't be surprised if I make mistakes (even if there were plenty of them...)
second, you can be helpful instead of wondering why my formatting looks weird to you.

Now, can you say what's wrong with those two first lines or you'd rather just point out the extra spaces in the code?

I didn't get past the first two lines, where you allocate memory for a pointer using the size of another pointer that you then dereference even if it doesn't point at anything large enough. Past that, I literally could not read.

Trying to see what that one variable does is almost impossible because all your variables are named something similar. Even an in-page search just captures all your other variables too because of the similar names. I can't sensibly make head nor tail of anything past the first "else" without literally renaming everything and formatting your code.

And you insert spaces into things which totally breaks them up and, from how I read it, kills the association with what they are part of.

Code:

---

temp_flight_head ->info ->passengers->info->id < temp_passenger ->info ->id && temp_flight_head ->info ->passengers->next

---

is quite horribly unreadable, for instance, even if I could guess what half those things do because it's hard to physically READ, not just interpret. I've never seen that style of pointer arrow before and didn't even know it was legal to leave a gap. I'm not saying it's *WRONG* (like some parts of your code), but it's certainly harder to read than necessary. And when you code dump two quite large functions, you're expecting people to read and help - you seem to take offence that I find it difficult to read your code.

P.S. I started programming nearly 2 decades ago. I still balls things up and write unreadable code sometimes. Chill out.

Please show what include files you are including. In a C program you shouldn't cast the return value from malloc. Are you sure that the memory you are freeing has actually been allocated by malloc? In the following snippet:

Code:

---

void cancel_ticket(flights_list* flights, int number, int id)
{
    flights_list* passenger_flights;
    passengers_list* passengers_on_flight;
    passenger* passenger;
    passengers_list* temp_list;
    while (flights ->info->number != number)
        flights = flights ->next;
    passengers_on_flight = flights ->info->passengers;
    if (passengers_on_flight ->info ->id != id)
    {
        while (passengers_on_flight ->next ->info ->id != id)
            passengers_on_flight = passengers_on_flight ->next;
        temp_list = passengers_on_flight ->next;
        passenger = passengers_on_flight ->next->info;
        passenger_flights = passengers_on_flight->next ->info ->flights;
        passengers_on_flight ->next = passengers_on_flight ->next ->next;
        free (temp_list);
    }

---

Where are you allocating memory for passengers_on_flight? Where do you actually fill in these data structures?

Jim

Quote:

---

Originally Posted by ledow

I didn't get past the first two lines, where you allocate memory for a pointer using the size of another pointer that you then dereference even if it doesn't point at anything large enough. Past that, I literally could not read.

Trying to see what that one variable does is almost impossible because all your variables are named something similar. Even an in-page search just captures all your other variables too because of the similar names. I can't sensibly make head nor tail of anything past the first "else" without literally renaming everything and formatting your code.

And you insert spaces into things which totally breaks them up and, from how I read it, kills the association with what they are part of.

Code:

---

temp_flight_head ->info ->passengers->info->id < temp_passenger ->info ->id && temp_flight_head ->info ->passengers->next

---

is quite horribly unreadable, for instance, even if I could guess what half those things do because it's hard to physically READ, not just interpret. I've never seen that style of pointer arrow before and didn't even know it was legal to leave a gap. I'm not saying it's *WRONG* (like some parts of your code), but it's certainly harder to read than necessary. And when you code dump two quite large functions, you're expecting people to read and help - you seem to take offence that I find it difficult to read your code.

P.S. I started programming nearly 2 decades ago. I still balls things up and write unreadable code sometimes. Chill out.

---

Excellent, thank you very much.
I didn't mean to sound offended and I am more than open for criticism. I just didn't like the way you shot it out without any explanation.
Anyway, you are correct about the malloc. I thought I was allocating a "passengers_list" struct but I was actually allocating memory suitable only for a pointer (correct?).
Fixed it and it works well.

About the spaces - I get what you say and I think you are right...in previous assignments I tended to use more spaces and enters but it does look rather confusing when using pointers.
And the names...well, I do need to get more creative...

Allow me to introduce you to the Law of Demeter.
It basically that any code like a.b.c or a->b->c is bad, and code like:

Quote:

---

Originally Posted by tmrsgv

Code:

---

temp_flight_head->info->passengers->info->id

---

---

Well that's horrifically bad.

Please please read that link and think about how you can apply that principle to your program. If you don't learn this now, you will hold yourself back a lot, causing all sorts of problems for yourself.

One of my most common bad programming habits is to pull things out to use them singularly rather than have a "pointer-train" like you have (it does look like a train!). My alternative is something I call it "this-itis".

i.e. if I am looping over several schools, each of which has classes, each of which has students, then I tend to make it so I end up with:

Code:

---

this_school = all_schools[i];
...
this_class = this_school->classes[i];
...
this_student = this_class->students[i];
...

---

and then long-winded tests become something more manageable:

Code:

---

if(this_student->dead  && this_student->funded);
{
    printf("Aw\n");
    this_school->budget[2013]--;
};

---

It's quite common to see this structure in programs that I write, even if I hardly ever use the "this_" variable (it's also my most common source of "variable not used" warnings as the code evolves). But, if you're using pointers, the compiler optimises all those intermediate steps away anyway, so you don't lose anything and only gain (some) readability.

And, yes, that was what I was referring to with the pointer. Again, I haven't checked anything past that so there may be other problems (and also, I'm not infallible!).