[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