On Fri, Dec 3, 2021 at 3:53 PM Ilya Maximets <[email protected]> wrote: > > 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.
Ack, I'll not introduce calls from endian.h here. I'll investigate if I can find other more human readable and still portable ways of dealing with this if that is a concern for the current proposal. > > > >> + 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. Thank you for pointing that out, I will update the use of the `sizeof` operator in this file as part of this fix. I see that this is also mentioned in the style guide, I will make sure to study it in more detail. Cheers! -- Frode Nordahl > > Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
