On 11/25/21 19:49, Mike Pattrick wrote:
> Hello Frode,
> 
> This patch does appear to fix the test case on big-endian systems.
> 
> On Fri, 2021-11-19 at 06:08 +0100, Frode Nordahl wrote:
>> The netlink policy unit test contains test fixture data that is
>> subject to endianness and currently fails on big endian systems.
>>
>> Add helper that ensures correct byte order for the affected data.
>>
>> Fixes: bfee9f6c0115 ("netlink: Add support for parsing link layer address.")
>> Signed-off-by: Frode Nordahl <[email protected]>
>> ---
>>  tests/test-netlink-policy.c | 60 +++++++++++++++++++++++--------------
>>  1 file changed, 38 insertions(+), 22 deletions(-)
>>
>> diff --git a/tests/test-netlink-policy.c b/tests/test-netlink-policy.c
>> index 5f2bf7101..3050faebc 100644
>> --- a/tests/test-netlink-policy.c
>> +++ b/tests/test-netlink-policy.c
>> @@ -32,6 +32,22 @@ _nla_len(const struct nlattr *nla) {
>>      return nla->nla_len - NLA_HDRLEN;
>>  }
>>  
>> +/* The header part of the test fixture data is subject to endianness.  This
>> + * helper makes sure they are put into the buffer in the right order. */
>> +static void
>> +prepare_ofpbuf_fixture(struct ofpbuf *buf, uint8_t *data, size_t size) {
>> +    uint16_t hword;
>> +
>> +    ovs_assert(size > 4);
>> +
>> +    ofpbuf_init(buf, size);
>> +    hword = data[0] | data[1] << 8;
> 
> This is pretty minor, but what do you think about using htobe16
> instead?

This doesn't sound very portable, so it's probably better to not use.

> 
>> +    ofpbuf_put(buf, &hword, sizeof(hword));

Frode, please, don't parenthesize the argument of 'sizeof' if that
is not necessary.  I know that it was like that in this file before,
but since you're touching almost all the sizeofs in this file, it
would be great to fix the style there as well.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to