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
