>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).
Personaly, I agree with Andrew Copper to remove the be*_to_cpup helpers as current
implemetation is just a wrapper, like
#ifndef __arch__swab16p
# define __arch__swab16p(x) __arch__swab16(*(x))
#endif
With be*_to_cpup been removed, the interface keeps simple and clear, and callers
are dereference the pointer explictly.
I am very happy to see the three choices, hope we can reach an agreement about this soon.
Cherrs,
---
Lin
