Re: [PATCH] net_sched: force endianness annotation

[Nicholas Mc Guire]

On Mon, Apr 29, 2019 at 12:11:18PM +0100, Edward Cree wrote:
> On 29/04/2019 11:44, Nicholas Mc Guire wrote:
> > be16_to_cpu((__force __be16)val) should be a NOP on big-endian as well
> Yes.  But it's semiotically wrong to call be16_to_cpu() on a cpu-endian
>  value; if the existing behaviour is desired, it ought to be implemented
>  differently.
> > The problem with using swab16 is that it is impating the binary significantly
> > so I'm not sure if the change is really side-effect free
> It's not; it changes the behaviour.  That's why I brought up the question
>  of the intended behaviour ??? it's unclear whether the current (no-op on BE)
>  behaviour is correct or whether it's a bug in the original code.
> Better to leave the sparse error in place ??? drawing future developers'
>  attention to something being possibly wrong here ??? than to mask it with a
>  synthetic 'fix' which we don't even know if it's correct or not.
> 
> > but I just am unsure if
> > -                   val = be16_to_cpu(val);
> > +                   val = swab16(val);
> > is actually equivalent.
> If you're not sure about such things, maybe you shouldn't be touching
>  endianness-related code.  swab is *not* a no-op, either on BE or LE.

Well the only way to understand it is to try to understand it by reviewing
the implementatoins - which is whyt I'm currently doing - the principle 
issues are clear I think - following the details of the macro-chains is
not always that clear. From looking at the code history it does seem correct
which is why it seemed reasonable to remove the sparse warning and doing
so with a patch that does not change the binary seems the safest.

thx!
hofrat