0

This is a school assignment and I have most of it under control, but there is a small part that is creating a memory leak and I have no more ideas on how to fix it.

We have created a memory allocator, and the trouble part are this two functions.

This first one cannot be modified

void destroy (){
  free():
  tot_alloc = 0; }

This second one is the one I'm working on

void free(){
  munmap(pool, PAGE_SIZE);
  Page* f = this;
  f = f->prev;
  if (f != NULL)
    f->destroy();
}

I have written all of the free() function and was asked in the assignment to call destroy(). I realize that this function is not destroying the first "this" because immediately is going to f->prev but I have no clue how to make it first destroy this and the go to prev.

I hope this is not a too silly question.

Many thanks!

Nico

NicoTek
  • 1,127
  • 1
  • 14
  • 34
  • 1
    The thing you mentioned is called destructor - not deconstructor. – Simon Nov 06 '11 at 14:24
  • 3
    Did you follow the [rule of three](http://stackoverflow.com/questions/4172722/)? – fredoverflow Nov 06 '11 at 14:29
  • 1
    It's really not clear what this code is supposed to do. Is 'free' supposed to call 'destroy' on every element of a doubly-linked list? Or is it supposed to remove this page from a doubly-linked list? Why are we destroying the page before this page? – David Schwartz Nov 06 '11 at 14:43
  • @DavidSchwartz free() is supposed to call destroy() on every element of single-linked list. And the reason why we are destroying the page before this page is because i'm not able to fix that, and that is my question. how can I destroy this page and then follow to destroy all of the other pages – NicoTek Nov 06 '11 at 14:47
  • You really mean the 'free' member function for one entry in a singly-linked list is supposed to free every entry on that list? (What other pointers do you have? Do you have a pointer to the first entry on the list? The last? Do members have 'next' pointers as well as 'prev' poitners?) – David Schwartz Nov 06 '11 at 14:49
  • @DavidSchwartz I have a global variable Page* pages that points to the last item in the list. And the members don't have 'next' pointers and I'm not allowed to add that. and i have also the size of the Page – NicoTek Nov 06 '11 at 14:56
  • Ahh, okay. So you have to find the page after this one by walking the list. – David Schwartz Nov 06 '11 at 14:58

1 Answers1

2

To remove an element from a singly-linked list, you must walk to that element to find the element before it in the list. Then you have to 'stitch' around it. Like this:

void free()
{ // Note: This has no error checking!
    Page *f = tail, *next_f = NULL;
    while(f != this)
    { // find this node to find the node after this node
        next_f = f;
        f = f->prev;
    }

    // found it. Now, next_f is the node after this node.
    // close the chain so this link can go away
    if (next_f == NULL) // there is no node after us, we must be the tail
       tail = prev; // the tail is now whatever node is after us
    else // in the node after us, the node before it is now the node before us
       next_f->prev = prev; 

    destroy(); // we are unlinked from the chain so can now be freed
}
David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • I replaced what you call tail for my pointer pages and I'm getting a seg-fault... I like your idea and it has open some possibilities but still not sure how to destroy the last page and the all of the others. – NicoTek Nov 06 '11 at 15:51
  • Add error checking. It's possible some other code corrupts your list. For example, after `f = f->prev`, `f` cannot possibly be NULL unless the list is corrupt. Check that. In the `if (next_f == NULL)` clause, `tail` must be equal to `this`. Check that. – David Schwartz Nov 06 '11 at 15:53
  • I think my destructor might be fine after all, I still have a leak but it might be in some other part of the program. – NicoTek Nov 08 '11 at 01:53