CODEWITHSUNDEEP
cmemory-managementfree
tanbog
tanbog

Reputation: 610

Invalid Free or corruption out

I am new to C and totally lost by this problem. This is a homework assignment to implement something like the pagerank algorithm in C.

I am trying to record the links from other pages by way of a 2D pointer-pointer array. My program works just fine and happily calculates pagerank for large sets of links, however, whenever I try to free my links array I get an "invalid free" error.

example code:

struct webpage {
  char name[20];
  int links_out;
  struct webpage **links_in; //to hold pointers to pages.
  int index; //stores the position in the pre-sorted array
             // as I have to print it out in this order
};

static struct webpage *pages = NULL;

This is my data structure. After reading in some basic variables like the number of pages (npages) I then allocate memory

pages = (struct webpage *)calloc(npages, sizeof(struct webpage));

As I read in each webpage I allocate the internal 2d array links_in as follows

pages[counter].links_in = (struct webpage **)calloc(npages, sizeof(struct webpage *));

and then each page inside:

for(i =0; i< npages; ++i)
pages[counter].link_in[i] = (struct webpage *)calloc(1, sizeof(struct webpage));

I sort my array of webpages. Then read in the link information and binary search my array with bsearch to get a pointer to each desired page. 

struct webpage *temp_in;
struct webpage *temp_out;

temp_in = bsearch(temp_str, pages, npages, sizeof(struct webpage), struct_cmp_by_name);
temp_out = bsearch(temp_str2, pages, npages, sizeof(struct webpage), struct_cmp_by_name);

Then I assign

temp_in->links_in[temp_out->index] = temp_out;

This all works great and I am able to access all the desired data to calculate pagerank. Once I am done I try to free the memory as follows:

for(int i = 0; i < npages; ++i){
  for(int k = 0; k < npages; ++k){
     if(pages[i].links_in[k] != NULL){
          free(pages[i].links_in[k]); //this line is causing the error
          pages[i].links_in[k] = NULL;
     }
  }
 free(pages[i].links_in);
}

The call to free inside the k loop throws an invalid free (or a double free corruption out) error the very first time it is called.

I have been over the code with gdb and things seem to be pointing to the right thing. gdb list that line as the point where the program breaks.

valgrind says much the same:

Invalid free() / delete / delete[]
at 0x4A05187: free (vg_replace_malloc.c:325)
by 0x4009C4: memory_dump (pagerank.c: 87)  // this is the line free(pages[i].links_in[k];
by 0x4013F2: seq_check_condition (pagerank.c:355)
by 0x40162F: main (pagerank.c:418)

At no point do I delete/free that (or any memory tbh) memory before that point.

thanks for any suggestions.

Upvotes: 0

Views: 669

Answers (3)

phoxis
phoxis

Reputation: 61920

I think you do not need to do the following

for(i =0; i< npages; ++i)
    pages[counter].link_in[i] = (struct webpage *)calloc(1, sizeof(struct webpage));

This is because for each page you allocate a new webpage pointer. The pages[counter].link_in[i] is used to hold the pointer to a webpage , ie. it should point to an already existing and allocated page. So you need to only assign the address of that webpage instance.

I think the problem is here:

Change your this code:

for(int i = 0; i < npages; ++i){
  for(int k = 0; k < npages; ++k){
     if(pages[i].links_in[k] != NULL){
          free(pages[i].links_in[k]); //this line is causing the error
          pages[i].links_in[k] = NULL;
     }
  }
 free(pages[i].links_in);
}

To this:

for(int i = 0; i < npages; ++i){
   free (pages[i].links_in);
}
free (pages);

This is because the links the pages[i].links_in[x] holds are pointers to some pages[j], and because a particular pages[j] address can occur in pages[i].links_in[x] for any number of i, so potentially the address of the pages[j] occurs in multiple places, which is attempted to be freed by your routine in the line free(pages[i].links_in[k]);.

Please let us know if this helps.

Upvotes: 0

nmichaels
nmichaels

Reputation: 50971

It looks like pages[i].links_in[k] should be pages[i].link_in[k].

Upvotes: 0

sje397
sje397

Reputation: 41822

My guess is this is the problem:

temp_in->links_in[temp_out->index] = temp_out;

It seems you've already allocated memory for all your 'links_in' pointers - then you're changing that pointer to point to another bit of memory you've allocated.

Maybe you want something like:

memcpy(temp_in->links_in[temp_out->index], temp_out, sizeof(struct webpage));

...or a bit of a redesign?

Upvotes: 1

Related Questions