Am 28.07.2014 19:17, schrieb Jeff King:
> Hi Karsten,
> The hashmap_entry documentation claims:
>   `struct hashmap_entry`::
>       An opaque structure representing an entry in the hash table,
>       which must be used as first member of user data structures.
>       Ideally it should be followed by an int-sized member to prevent
>       unused memory on 64-bit systems due to alignment.
> I'm not sure if the statement about alignment is true. If I have a
> struct like:
>     struct magic {
>           struct hashmap_entry map;
>           int x;
>     };
> the statement above implies that I should be able to fit this into only
> 16 bytes on an LP64 system. But I can't convince gcc to do it. And I
> think that makes sense, if you consider code like:
>    memset(&, 0, sizeof(struct hashmap_entry));
> The sizeof() has to be the same regardless of whether the hashmap_entry
> is standalone or in another struct, and therefore must be padded up to
> 16 bytes. If we stored "x" in that padding in the combined struct, it
> would be overwritten by our memset.

The struct-packing patch was ultimately dropped because there was no way
to reliably make it work on all platforms. See [1] for discussion, [2] for
the final, 'most compatible' version.

> Am I missing anything? If this is the case, we should probably drop that
> bit from the documentation.

Hmmm. Now that we have "__attribute__((packed))" in pack-bitmap.h, perhaps
we should do the same for stuct hashmap_entry? (Which was the original
proposal anyway...). Only works for GCC, but that should cover most builds
/ platforms.

Btw.: Using struct-packing on 'struct bitmap_disk_entry' means that the
binary format of .bitmap files is incompatible between GCC and other
builds, correct?

> It's possible that we could get around it by
> embedding the hashmap_entry elements directly into the parent struct,

Already tried that, see [3].


To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to