Issues with pointer and freeing its associated memory

[tytelizgal]

I am implementing a vector in C and am having trouble with the delete function. 

1) here is my vector struct: 

typedef struct {
  void* elems;
  int elemSize;
  int logLength;
  int allocLength;
  VectorFreeFunction vfree;
} vector;

2) here is the function from which i call the delete function (in a test file):
static void TestInsertDelete(vector *alphabet)
{
  char ch = '-';
  int i;
  for (i = 3; i <= VectorLength(alphabet); i += 4) // Insert dash every 4th char 
    VectorInsert(alphabet, &ch, i);
  fprintf(stdout, "\nAfter insert dashes: ");
  VectorMap(alphabet, PrintChar, stdout);
  
  for (i = 3; i < VectorLength(alphabet); i += 3) // Delete every 4th char 
    VectorDelete(alphabet, i);
  fprintf(stdout, "\nAfter deleting dashes: ");
  VectorMap(alphabet, PrintChar, stdout);
    
  ch = '!';
  VectorInsert(alphabet, &ch, VectorLength(alphabet));
  VectorDelete(alphabet, VectorLength(alphabet) - 1);
  fprintf(stdout, "\nAfter adding and deleting to very end: ");
  VectorMap(alphabet, PrintChar, stdout);
}

3) Finally, here is my vector delete implementation that is causing an error (an "abort (core dumped)" error, it also says that there is an invalid pointer):

//VectorDelete will use the client-supplied free function to free the
//actual memory pointed to by the elements in the vector.
void VectorDelete(vector *v, int position)
{
  fprintf(stdout,"gets  to delete \n");
  assert (position >=0);
  assert (position <= v->logLength-1);
  assert (v != NULL);
  void* movept = (char*) v-> elems + position * v->elemSize;
  assert (movept != NULL);
  free(movept);
  if(v->vfree == NULL)free(movept);
  else v->vfree(movept);
  memmove(movept, (char*) movept + v-> elemSize, v-> logLength* v-> elemSize - position*v->
   elemSize);
  v->logLength--;
}


ERROR GENERATED: 
*** glibc detected *** ./vector-test: free(): invalid pointer: 0x0804c00b ***
======= Backtrace: =========
/lib/tls/i686/cmov/libc.so.6[0xb7eaf7cd]
/lib/tls/i686/cmov/libc.so.6(cfree+0x90)[0xb7eb2e30]
./vector-test[0x80497f3]
./vector-test[0x8048a6c]
./vector-test[0x8048c7c]
./vector-test[0x80492f9]
/lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xdc)[0xb7e5debc]
./vector-test[0x8048601]
======= Memory map: ========
08048000-0804b000 r-xp 00000000 00:13 1354564242  /afs/ir.stanford.edu/users/l/i/lizb1/myassn3/assn-3-vector-hashset/vector-test
0804b000-0804c000 rw-p 00002000 00:13 1354564242  /afs/ir.stanford.edu/users/l/i/lizb1/myassn3/assn-3-vector-hashset/vector-test
0804c000-0806d000 rw-p 0804c000 00:00 0          [heap]
b7d00000-b7d21000 rw-p b7d00000 00:00 0 
b7d21000-b7e00000 ---p b7d21000 00:00 0 
b7e47000-b7e48000 rw-p b7e47000 00:00 0 
b7e48000-b7f83000 r-xp 00000000 08:01 1176036    /lib/tls/i686/cmov/libc-2.5.so
b7f83000-b7f84000 r--p 0013b000 08:01 1176036    /lib/tls/i686/cmov/libc-2.5.so
b7f84000-b7f86000 rw-p 0013c000 08:01 1176036    /lib/tls/i686/cmov/libc-2.5.so
b7f86000-b7f89000 rw-p b7f86000 00:00 0 
b7f89000-b7f94000 r-xp 00000000 08:01 1172800    /lib/libgcc_s.so.1
b7f94000-b7f95000 rw-p 0000a000 08:01 1172800    /lib/libgcc_s.so.1
b7f95000-b7f98000 rw-p b7f95000 00:00 0 
b7f98000-b7fb1000 r-xp 00000000 08:01 1172757    /lib/ld-2.5.so
b7fb1000-b7fb3000 rw-p 00019000 08:01 1172757    /lib/ld-2.5.so
bffb2000-bffc8000 rw-p bffb2000 00:00 0          [stack]
ffffe000-fffff000 r-xp 00000000 00:00 0          [vdso]
Abort (core dumped)
pod8:~/myassn3/assn-3-vector-hashset>

[Dino]

This is a bug:

for (i = 3; i < VectorLength(alphabet); i += 3) // Delete every 4th char

[tytelizgal]

Hmm taht was given in the test code (I didn't write that) I think 3 is still valid because when you go through and delete, the indices all drop by 1 so the next one (the one that used to be 4 away) is now only 3 away.

Either way, I tried changing it to i+=4 and still got the exact same error.

[matsp]

  void* movept = (char*) v-> elems + position * v->elemSize;
  assert (movept != NULL);
  free(movept);
  if(v->vfree == NULL)free(movept);
  else v->vfree(movept);
  memmove(movept, (char*) movept + v-> elemSize, v-> logLength* v-> elemSize - position*v->
   elemSize);
  v->logLength--;

So, you free movept, then free it again if v->vfree is NULL, and hten you store data into the memory it used to point to - seems very bad to me.

Edit: I also someohow suspect that:

  void* movept = (char*) v-> elems + position * v->elemSize;

doesn't actually come up with the originally allocated memory location.



--
Mats

[Dino]

Well, that obviously wasn't the solution to the problem you reported - it was just pointing out an inconsistency with your logic and your comment. And yes, you do need to start with 3, just like you are, but then you add 4 each iteration if you want to delete every 4th element.

We really don't have enough information for your initial question.

If this is the line of code in error:

 free(movept);

then my initial guess is that you are deriving the pointer arbitrarily and you are not picking up the same address that was previously malloc()'ed.

[tytelizgal]

I am still getting the same issue...I took the suggestion into account that I should be freeing the exact pointer that I malloc'd...

//VectorDelete will use the client-supplied free function to free the
//actual memory pointed to by the elements in the vector.
void VectorDelete(vector *v, int position)
{
  fprintf(stdout,"\n gets  to delete \n");
  assert (position >=0);
  assert (position <= v->logLength-1);
  assert (v->elems != NULL);
  v-> elems = (char*) v-> elems + position* v-> elemSize;
  assert (v-> elems != NULL);
  fprintf(stdout,"ok gets to the place \n");
  if(v->vfree == NULL) free (v->elems);
  else v->vfree (v-> elems);
  // void* movept = (char*) v-> elems + position * v->elemSize;
  // assert (movept != NULL);
  //there is some error with the pointer itself, not what it does 
  //fprintf(stdout, "\n woot");
  //if(v->vfree == NULL)free(movept);
  //  else v->vfree(movept);
  void* movept = v-> elems;
   memmove(movept, (char*) movept + v-> elemSize, v-> logLength* v-> elemSize - position*v->
   elemSize);
  if(v-> logLength > 0)v->logLength--;
}

Any other ideas as to what can be going wrong?

[matsp]

v-> elems = (char*) v-> elems + position* v-> elemSize;
assert (v-> elems != NULL);
fprintf(stdout,"ok gets to the place \n");
if(v->vfree == NULL) free (v->elems);

Unless position is zero, I'm pretty sure v->elems after teh calculation isn't going to point to what you allocated with malloc - we could confirm this by looking at the allocation code.

--
Mats

[tytelizgal]

Here is my allocation code from vector new:

Yes, I was a little iffy on moving around the v-> elems pointer as I thought it should always point to the beginning of my array.

void VectorNew(vector *v, int elemSize, VectorFreeFunction freeFn, int initialAllocation)
{
  //purpose of vect free function?
  assert(elemSize >  0);
  v->logLength = 0;
   if(initialAllocation ==0) initialAllocation = INT_ALL_SIZE;
  v->allocLength = initialAllocation;
  v->elemSize = elemSize;
  v->elems = malloc (v-> allocLength * v-> elemSize);
  assert (v->elems != NULL);
  fprintf(stdout,"done calling vector new\n");
}

LAST CHANCE TO READ THIS!!!!!
http://cboard.cprogramming.com/showthread.php?t=25765

[Dino]

OK, so the beginning of the malloc()'ed storage is always at v->elems. Therefore, the free() should only be from the same address. Don't get creative on your free()s.

[tytelizgal]

But I have to free an element that is not at the beginning of the vector...so how would I do that using v->elems?

[tabstop]

But I have to free an element that is not at the beginning of the vector...so how would I do that using v->elems?

If you need to do that, then your entire data structure has been designed incorrectly the whole time. The whole point of your vector, as you have it, is that the whole thing is one monolithic block (that you can access randomly). Either you keep the whole thing or none at all.