CODEWITHSUNDEEP
cmemoryvalgrind
Uncle Dolan
Uncle Dolan

Reputation: 65

Valgrind reports a memory error when trying to free a malloc'ed struct

Valgrind always nags that there is a memory error when a previously malloc'ed struct is free'd. The struct looks as follows:

typedef struct bullet
{
  int x, y;
  struct bullet * next;
} BULLET;

... and I allocate the memory by using

BULLET * b;
b = malloc(sizeof(BULLET)); // sizeof(BULLET) is 16

and later on, the struct is free'd by simply calling free(b);. Valgrind however doesn't seem to be satisfied by this, and so it tells me

==2619== Invalid read of size 8
==2619==    at 0x40249F: ctrl_bullets (player.c:89)
==2619==    by 0x405083: loop_game (game.c:305)
==2619==    by 0x406CCA: main (main.c:47)
==2619==  Address 0x5b8d818 is 8 bytes inside a block of size 16 free'd
==2619==    at 0x4C29A9E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2619==    by 0x402E04: rm_bullet (player.c:329)
==2619==    by 0x402485: ctrl_bullets (player.c:95)
==2619==    by 0x405083: loop_game (game.c:305)
==2619==    by 0x406CCA: main (main.c:47)

Of course I can't just allocate only 8 bytes, because that would be the size needed to store the pointer, and not the size of the struct - so why does Valgrind keep telling me that there is an error?

Edit: Some more code which could be relevant ...

void
ctrl_bullets(WINDOW * w_field, BULLETLIST * lb)
{
  if (lb->num > 0)
  {
    BULLET * b;

    for (b = lb->head; b != NULL; b = b->next) // player.c:89
    {
      if (b->x > CON_FIELDMAXX)
      {
        write_log(LOG_DEBUG, "Bullet %p is outside the playing field; x: %d; "
                  "y: %d\n", (void *) b, b->x, b->y);
        rm_bullet(w_field, lb, b);
      }
      else
      {
        mv_bullet(w_field, b);
      }
    }
  }
}

Upvotes: 1

Views: 7306

Answers (2)

Marco
Marco

Reputation: 2897

The problem is that you free b, but then try to access b->next

The error that valgrind tells you is that you are accessing an 8 byte block (the NEXT pointer) inside a 16 byte block that has been free'd.

If you free b, you can't access to b->next. Just store it in a tmp variable :P

(also, remember to set b to null after freeing it so you don't have a Dangling Pointer)

Upvotes: 5

Jens Gustedt
Jens Gustedt

Reputation: 78903

8 bytes inside BULLET is most likely the field next. You probably have kept &next somewhere and reuse that in ctrl_bullets (player.c:89). But since you don't show us that code, we can't say much more.

Edit: or perhaps we can guess that your next pointer forms a linked list and that you haven't set the next pointer of another element to 0 when doing the free.

Upvotes: 0

Related Questions