[asterisk-dev] changing sizeof to __alignof__

Walter Doekes walter+asterisk-dev at osso.nl
Fri Nov 4 03:15:42 CDT 2011


Kevin wrote:
>> -    char base[0];                /*!<  storage space for the fields */
>> +    char base[0] __attribute__((aligned(sizeof(ast_string_field_allocation)))); /*!<  storage space for the fields */
>
> Sorry I didn't get to this while it was still on ReviewBoard; I agree with these changes, they are the right thing to do. The only additional comment that I would make is that I would prefer to use "alignof(ast_string_field_allocation)" instead of "aligned(sizeof(...))". This allows the compiler to tell you how an instance of that type should be aligned, which *might* not be on a boundary the same as its size. Granted, for all common platforms the alignment of a 2-byte integer is also 2 bytes... but it's conceivable that some platform might want to align 2-byte integers on 4-byte boundaries in the future.

I looked at that, but that would be a matter of not only replacing this 
sizeof, but replacing all "sizeof(ast_string_field_allocation)" with 
"__alignof__(ast_string_field_allocation)".

I'm not particularly fond of adding __special_compiler_magic__ all 
around the source.

So, I would still like to fix it, but then I have the choice between:


Option A: Using autoconf-stuff and a header file to #define alignof to 
__alignof__() or sizeof() if there is no __alignof__.

Replacing all of "sizeof(ast_string_field_allocation)" with 
"alignof(ast_string_field_allocation)" is acceptable to me.


Option B: Replace the macro's I added -- with something that takes a 
type instead of a size -- so the __alignof__ keyword can stay in the 
header files (utils.h and stringfields.h) where they're less visible.

I would have to replace the two macro's (ast_add_and_make_multiple_of, 
ast_make_multiple_of) from taking a size to taking a type; renaming them 
to: ast_make_room_for and ast_align_for respectively.


Option C: both, where the "alignof" is contained to utils.h and 
stringfields.h and "__alignof__" is only found in something like compat.h.


Preferences anyone?


Regards,
Walter Doekes



More information about the asterisk-dev mailing list