On 10/20/2014 08:42 PM, Lennart Poettering wrote:
> On Thu, 16.10.14 09:50, Michal Schmidt (mschm...@redhat.com) wrote:
>> +/* Fields that all hashmap/set types must have */
>> +struct HashmapBase {
>> + const struct hash_ops *hash_ops; /* hash and compare ops to use */
>> +
>> + union _packed_ {
>> + struct indirect_storage indirect; /* if has_indirect */
>> + struct direct_storage direct; /* if !has_indirect */
>> + };
>> +
>> + uint8_t type:2; /* HASHMAP_TYPE_* */
>
> Should probably be a named enum (see above)
>
>> + uint8_t has_indirect:1; /* whether indirect storage is
>> used */
>
> Should really be a "bool", no?
Interestingly, changing this confuses pahole greatly.
With all the three bitfields declared as uint8_t pahole showed the struct
layout sanely:
struct HashmapBase {
const struct hash_ops * hash_ops; /* 0 8 */
union {
struct indirect_storage indirect; /* 39 */
struct direct_storage direct; /* 39 */
}; /* 8 39 */
uint8_t type:2; /* 47: 6 1 */
uint8_t has_indirect:1; /* 47: 5 1 */
uint8_t n_direct_entries:3; /* 47: 2 1 */
/* size: 48, cachelines: 1, members: 5 */
/* bit_padding: 2 bits */
/* last cacheline: 48 bytes */
};
With "type" changed to a named enum, "has_indirect" to bool,
and "n_direct_entries" to unsigned, pahole says:
base_type__name_to_size: base_type __unknown__
base_type__name_to_size: base_type __unknown__
die__process_inline_expansion: tag not supported (INVALID)!
base_type__name_to_size: base_type __unknown__
...
struct HashmapBase {
const struct hash_ops * hash_ops; /* 0 8 */
union {
struct indirect_storage indirect; /* 39 */
struct direct_storage direct; /* 39 */
}; /* 8 39 */
/* Bitfield combined with next fields */
enum HashmapType type:2; /* 44: 6 4 */
/* XXX 22 bits hole, try to pack */
/* Bitfield combined with next fields */
_Bool has_indirect:1; /* 47: 5 1 */
/* Bitfield combined with previous fields */
__unknown__ n_direct_entries:3; /* 44: 2 0 */
/* size: 48, cachelines: 1, members: 5 */
/* bit holes: 1, sum bit holes: 22 bits */
/* padding: 4 */
/* bit_padding: 252 bits */
/* last cacheline: 48 bytes */
/* BRAIN FART ALERT! 48 != 48 + 0(holes), diff = 0 */
};
But as far as I can tell, generated code looks OK and it still runs
fine, so I won't worry about it.
Michal
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
