On 3 Mar 2026, at 15:23, Eelco Chaudron wrote:
> On 9 Feb 2026, at 14:29, Eli Britstein wrote:
>
>> From: Gaetan Rivet <[email protected]>
>>
>> Implicit padding bytes in a struct are not necessarily initialized.
>> Comparing such struct with memcmp might be wrong. To verify correctness
>> of such comparison, add a macro to statically assert whether a structure
>> contains padding.
>
> Thanks for the patch, Gaetan. Some small nits below.
I was thinking about this again after a coffee break, and it seems odd to
define a structure with a macro. Maybe the build assert should just follow
the structure definition? Something like the below more in line with the
Other BUILD_ASSERT_xxx macros:
struct key {
size_t idx;
bool b;
uint8_t pad[7];
};
BUILD_ASSERT_PACKED(struct key);
Ilya is already copied, but maybe he has some thoughts around this...
> //Eelco
>
>> Signed-off-by: Gaetan Rivet <[email protected]>
>> Co-authored-by: Eli Britstein <[email protected]>
>> Signed-off-by: Eli Britstein <[email protected]>
>> ---
>> include/openvswitch/compiler.h | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h
>> index 7dee9ec7a..961339944 100644
>> --- a/include/openvswitch/compiler.h
>> +++ b/include/openvswitch/compiler.h
>> @@ -218,6 +218,18 @@
>> #define OVS_PACKED(DECL) __pragma(pack(push, 1)) DECL __pragma(pack(pop))
>> #endif
>>
>> +/* Define a type with this macro to enforce during
>> + * compilation that it has no padding, i.e. packed.
>> + * It *DOES NOT* pack the type, only statically check
>> + * that it is already. Either the layout must be
>
> We use double spaces after the end of sentences.
>
>> + * made to fit, or explicit padding must be used.
>> + */
>
> I would personally add the comment ending on the same line. We seem to have
> mixed ways of doing this in this file, but when reviewing I'm trying to steer
> toward ending on the same line, unless the entire file does it differently.
>
>> +#define OVS_ASSERT_PACKED(TYPE, MEMBERS) \
>
> Since this is a build assertion, consider naming it like the other
> BUILD_ASSERT_*
> macros at the end of this file. Perhaps move it to that section as well for
> consistency?
>
>> + TYPE { \
>> + MEMBERS \
>> + }; \
>> + BUILD_ASSERT_DECL(sizeof(TYPE) == sizeof(OVS_PACKED(struct { MEMBERS
>> })))
>> +
>> /* OVS_ALIGNED_STRUCT may be used to define a structure whose instances
>> should
>> * aligned on an N-byte boundary. This:
>> * OVS_ALIGNED_STRUCT(64, mystruct) { ... };
>> --
>> 2.34.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
