Re: [PATCH V2] staging: fieldbus: anybus-s: force endiannes annotation

From: Al Viro
Date: Tue Apr 30 2019 - 10:27:52 EST


On Tue, Apr 30, 2019 at 09:32:20AM -0400, Sven Van Asbroeck wrote:
> On Tue, Apr 30, 2019 at 12:19 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > ... not that there's much sense keeping ->fieldbus_type in host-endian,
> > while we are at it.
>
> Interesting! Suppose we make device->fieldbus_type bus-endian.
> Then the endinan-ness conversion either needs to happen in
> bus_match() (and we'd have to convert endianness each time
> this function is called).
> Or, we make driver->fieldbus_type bus-endian also, then there
> is no need for conversion... but the driver writer has to remember
> to specify this in bus endianness:
>
> static struct anybuss_client_driver profinet_driver = {
> .probe = ...,
> .fieldbus_type = endian convert?? (0x0089),
> };
>
> Which pushes bus implementation details onto the
> client driver writer? Also, how to convert a constant
> to a specific endianness in a static initializer?

cpu_to_be16() or htons() - either will be fine there.
On little-endian you'll get
htons(0x0089) =>
___htons(0x0089) =>
__cpu_to_be16(0x0089) =>
((__force __be16)__swab16((0x0089))) =>
((__be16)(__builtin_constant_p((__u16)((0x0089))) ?
___constant_swab16((0x0089)) : __fswab16((0x0089))) =>
((__be16)(__builtin_constant_p((__u16)((0x0089))) ?
((__u16)((((__u16)((0x0089)) & (__u16)0x00ffU) << 8) |
(((__u16)((0x0089)) & (__u16)0xff00U) >> 8))) :
__fswab16((0x0089)))
and once the preprocessor has produced that, from compiler POV we have
a constant expression as argument of __builtin_constant_p(), so it
evaluates as true, reducing the whole thing to
((__be16)(((__u16)((((__u16)((0x0089)) & (__u16)0x00ffU) << 8) |
(((__u16)((0x0089)) & (__u16)0xff00U) >> 8))) )
i.e. (__be16)0x8900. On big-endian expansion will be different,
resulting in (__be16)0x0089...

IOW, you can use endianness convertors in static initializers; things
like
struct sockaddr_in addr = {.sin_addr.s_addr = htonl(0x7f000001),
.sin_port = htons(25),
.sin_family = AF_INET};
are fine kernel-side (from the compiler POV, that is - something
trying to speak SMTP in the kernel code would obviously be a bad sign).

As for having to remember - sparse will complain about endianness mismatches
in initializer...