Thread: "Heap corruption detected" Error

  1. #1
    Registered User
    Join Date
    Apr 2012
    Posts
    8

    "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...

  2. #2
    [](){}(); manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    *nullptr
    Posts
    2,657
    Use valgrind.
    (Or something like ElectricFence which I recently discovered).

  3. #3
    Registered User
    Join Date
    Mar 2011
    Posts
    546
    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.

  4. #4
    Registered User
    Join Date
    Apr 2012
    Posts
    8
    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?

  5. #5
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by tmrsgv View Post
    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.
    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.

  6. #6
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Quote Originally Posted by tmrsgv View Post
    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.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  7. #7
    Registered User
    Join Date
    Apr 2012
    Posts
    8
    Quote Originally Posted by anduril462 View Post
    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);
        }
        
    }

  8. #8
    Registered User ledow's Avatar
    Join Date
    Dec 2011
    Posts
    435
    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?

    - Compiler warnings are like "Bridge Out Ahead" warnings. DON'T just ignore them.
    - A compiler error is something SO stupid that the compiler genuinely can't carry on with its job. A compiler warning is the compiler saying "Well, that's bloody stupid but if you WANT to ignore me..." and carrying on.
    - The best debugging tool in the world is a bunch of printf()'s for everything important around the bits you think might be wrong.

  9. #9
    Registered User
    Join Date
    Apr 2012
    Posts
    8
    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?

  10. #10
    Registered User ledow's Avatar
    Join Date
    Dec 2011
    Posts
    435
    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.

    - Compiler warnings are like "Bridge Out Ahead" warnings. DON'T just ignore them.
    - A compiler error is something SO stupid that the compiler genuinely can't carry on with its job. A compiler warning is the compiler saying "Well, that's bloody stupid but if you WANT to ignore me..." and carrying on.
    - The best debugging tool in the world is a bunch of printf()'s for everything important around the bits you think might be wrong.

  11. #11
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    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

  12. #12
    Registered User
    Join Date
    Apr 2012
    Posts
    8
    Quote Originally Posted by ledow View Post
    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...

  13. #13
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    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 View Post
    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.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  14. #14
    Registered User ledow's Avatar
    Join Date
    Dec 2011
    Posts
    435
    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!).

    - Compiler warnings are like "Bridge Out Ahead" warnings. DON'T just ignore them.
    - A compiler error is something SO stupid that the compiler genuinely can't carry on with its job. A compiler warning is the compiler saying "Well, that's bloody stupid but if you WANT to ignore me..." and carrying on.
    - The best debugging tool in the world is a bunch of printf()'s for everything important around the bits you think might be wrong.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Heap corruption detected
    By yapic in forum C++ Programming
    Replies: 1
    Last Post: 05-26-2010, 09:38 AM
  2. Heap Corruption Detected -- Malloc issues
    By gp364481 in forum C Programming
    Replies: 4
    Last Post: 04-09-2010, 11:47 AM
  3. Heap corruption Detected Error
    By mrsirpoopsalot in forum C++ Programming
    Replies: 2
    Last Post: 10-26-2009, 03:59 PM
  4. Heap corruption detected. What does it mean?
    By franziss in forum C++ Programming
    Replies: 17
    Last Post: 07-23-2008, 02:50 AM
  5. "CWnd"-"HWnd","CBitmap"-"HBitmap"...., What is mean by "
    By L.O.K. in forum Windows Programming
    Replies: 2
    Last Post: 12-04-2002, 07:59 AM