Re: [PATCH v5 4/6] xen: Switch to byteswap

[Julien Grall <julien@xxxxxxx>]

Hi Andrew,

On 23/05/2022 16:38, Andrew Cooper wrote:
> On 23/05/2022 15:56, Julien Grall wrote:
> > Hi,
> >
> > On 23/05/2022 15:50, Lin Liu wrote:
> > > Update to use byteswap to swap bytes
> > > be*_to_cpup(p) is short for be*to_cpu(*p), update to use latter
> > > one explictly
> > But why?
> Because deleting code obfuscation constructs *is* the point of the cleanup.
>
> > I really don't have a suggestion on the comment because I disagree
> > (and AFAICT Jan as well) with the approach.
> Dropping the obfuscation has uncovered pre-existing bugs in the
> hypervisor.  The series stands on its own merit.
I am guessing you mean that we don't handle unaligned access? If so, yes 
I agree this helped with that.
> While I can't help if you like it or not, it really does bring an
> improvement to code quality and legibility.
>
> If you have no technical objections, and no suggestions for how to do it
> differently while retaining the quality and legibility improvements,
> then "I don't like it" doesn't block it going in.
And you don't like the existing code :). I am willing to compromise, but 
for that I need to understand why the existing code is technically not 
correct.
So far, all the arguments you provided in v3 was either a matter of 
taste or IMHO bogus.
Your taste is nor better nor worse than mine. At which, we need someone 
else to break the tie.
If I am not mistaken, Jan is also objecting on the proposal. At which 
point, we are 2 vs 1.
So there are three choices here:
  1) You find two others maintainers (including on Arm maintainer) to 
agree with you
  2) You provide arguments that will sway one of us in your side
  3) We keep be32_cpu*() (they are simple wrapper and I am willing to 
write the code).
Cheers,

--
Julien Grall