pydevd
pydevd

Reputation: 173

Pointer arithmetic for structures gives strange result

I have following function:

void deleteInventory( struct book** inventory )
{
    struct book* ptr = NULL;
    ptr = *inventory;

    while( length != 0)
    {
        free(ptr);
        length--;
        ++ptr;
    }
    free(inventory);
}

Structure:

#define MAX_LENGTH 64
#define ISBN_LENGTH 11

struct book
{
    char isbn[ISBN_LENGTH];
    char title[MAX_LENGTH];
    char author[MAX_LENGTH];
    int year;
    float price;
};

Total size of the structure: 147 bytes (11 + 64 + 64 + 4 + 4) sizeof(struct book) returns: 148 bytes

My inventory variable is array of structures and contains 3 records

Now, results of debugging in the function (addresses without high 2 bytes):

inventory[0] = 0x1350
inventory[1] = 0x14e0
inventory[2] = 0x1670

Difference in memory: 400 bytes, OK

On first iteration everything is OK, first record deletes without a problems

But after

++ptr;

ptr will contain: 0x13e4

inventory[0] - ptr = 148 bytes - correct difference according to structure's size

but it does not refers to next record, because address of the next record in memory: 0x14e0 and I'm getting corruption of the heap.

Any suggestions: why?

Upvotes: 2

Views: 177

Answers (4)

Raxvan
Raxvan

Reputation: 6505

I think you are deleting incorrectly from the inventory. After you free(ptr) then next memory block after ptr will probably be junk memory. Here i think is the correct way of deleting your inventory:

void deleteInventory( struct book** inventory )
{
    book** i =  inventory;
    while( length != 0)
    {
        free(*i);
        length--;
        ++i;//notice the difference (++i) instead of (++(*i))
    }
    free(inventory);
}

Upvotes: 6

zapatero
zapatero

Reputation: 131

The pointer arithmetic is not the issue... the problem is your assumptions on how the arrays are laid out is wrong.

I'm guessing you did something like:

  length = 3;
  inventory = (struct book**) malloc(sizeof(struct book *) * length);
  ...
  inventory[0] = (struct book*) malloc(sizeoof(struct book));
  inventory[1] = ... etc ;

Then you assign ptr to inventory[0];

  ptr = *inventory;

But then you assume that

 ++ptr == inventory[1];

and that is not true.

Upvotes: 1

PlasmaHH
PlasmaHH

Reputation: 16046

We can only speculate how inventory and its contents came to live, but since you free them individually, you probably have malloc()d them individually too. But you go through them via ptr and ptr++ which assumes contiguous allocated structure members, just like you would have in an array.

However nothing guarantees (and the average implementation will also not do it) that malloc() will return in any way contiguous storage for consecutive allocations.

This is why you see them more than the 148 bytes apart that you expect. Since probably inventory is an array to the allocated pointers, what you want to do is:

while( length != 0)
{
    free(*inventory);
    length--;
    ++inventory;
}

Upvotes: 2

Scott Mermelstein
Scott Mermelstein

Reputation: 15397

You've said that inventory is an array, and that's fine.

You've said that book takes up 148 bytes (when you count padding), and that's fine.

What you haven't said is that the pointers you place in inventory are allocated all at once, and the fact that you have an array of pointers implies to me that they're not.

However you create each of the books, it seems to be dynamically. There's no guarantee that the books will be contiguous, only that the pointers to them will be.

Change your function:

void deleteInventory( struct book** inventory )
{
    struct book* ptr = NULL;
    for (int i = 0; i < length; ++i) {
        ptr = inventory[i];
        free(ptr);
    }
    length = 0;
    free(inventory);
}

This will read the contiguous pointer-to-book array to delete all the books, without assuming that the books themselves are contiguously allocated.

Upvotes: 7

Related Questions