Re: [PATCH v5 15/18] whpx: arm64: implement -cpu host

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 8/19/25 9:08 AM, Daniel P. Berrangé wrote:
> On Tue, Aug 19, 2025 at 08:48:16AM -0700, Pierrick Bouvier wrote:
>> On 8/19/25 8:12 AM, Daniel P. Berrangé wrote:
>>> On Tue, Aug 19, 2025 at 04:06:45PM +0100, Peter Maydell wrote:
>>>> On Tue, 19 Aug 2025 at 16:04, Pierrick Bouvier
>>>> <pierrick.bouvier@linaro.org> wrote:
>>>>>
>>>>> On 8/19/25 6:24 AM, Peter Maydell wrote:
>>>>>> On Fri, 8 Aug 2025 at 07:55, Mohamed Mediouni <mohamed@unpredictable.fr> wrote:
>>>>>> Can you follow the QEMU coding style, please (here and elsewhere)?
>>>>>> Variables and function names should be all lower case,
>>>>>> and variable declarations go at the start of a C code
>>>>>> block, not in the middle of one.
>>>>>>
>>>>>
>>>>> In some cases, including in this function, I feel that the rule to
>>>>> declare variables at the start of a block is not really helpful, and is
>>>>> more related to legacy C than a real point nowadays.
>>>>> As well, it sometimes forces to reuse some variables between various sub
>>>>> blocks, which definitely can create bugs.
>>>>>
>>>>> Anyway, I'm not discussing the existing QEMU coding style, but just
>>>>> asking if for the current context, is it really a problem to declare
>>>>> variable here?
>>>>
>>>> The point of a coding style is to aim for consistency. QEMU
>>>> is pretty terrible at being consistent, but we should try.
>>>> The rule about variables at start of block is not because
>>>> some compilers fail to compile it, but because we think
>>>> it's overall more readable that way.
>>>
>>> There are also potential[1] functional problems with not declaring
>>> at the start of block, because if you have a "goto cleanup" which
>>> jumps over the line of the declaration, the variable will have
>>> undefined state when the 'cleanup:' block is running. This is
>>> something which is very subtle and easily missed when reading the
>>> code flow.
>>>
>>
>> This has nothing to do with where variables are declared, but where they are
>> assigned. The same issue can happen whether or not it's declared at the
>> start of a block.
>>
>> I suspect we use -ftrivial-auto-var-init precisely because we force
>> variables to be declared at start of the scope, i.e. where they don't have
>> any value yet. So, instead of forcing an explicit initialization or rely on
>> compiler warnings for uninitialized values, it was decided to initialize
>> them to 0 by default.
>>
>> If we declared them at the point where they have a defined semantic value,
>> this problem would not exist anyway, out of the goto_cleanup situation,
>> which has the same fundamental issue in both cases.
> 
> It really isn't the same issue when you compare
> 
>    void bar(void) {
>      char *foo = NULL;
> 
>      if (blah)
>         goto cleanup:
> 
>    cleanup:
>      if (foo)
>         ....
>    }
> 
> vs
> 
>    void bar(void) {
>      if (blah)
>         goto cleanup:
> 
>      char *foo = NULL;
>
>      ...some code...
>
>    cleanup:>      if (foo)
>         ....
>    }
> 
> The late declaration of 'foo' is outright misleading to reviewers.
> 
> Its initialization at time of declaration gives the impression
> that 'foo' has well defined value in the 'cleanup' block, when
> that is not actually true. In big methods it is very easy to
> overlook an earlier 'goto' that jumps across a variable declaration
> and initialization.
>

"Big" method is probably the issue there. If it's not possible to follow 
control flow in a given function, it's a strong hint there is a problem 
with its size, independently of any standard. And even though 
goto_cleanup is a legit pattern in C, I still don't get the argument 
about declaring variable far from their definition point in this case.
It seems that we are trying to solve the consequence without really 
understanding the root cause issue.

> Even if not all methods have this problem, the coding standards
> guide us into the habit of writing code that is immune from this
> kind of problem. That habit only forms reliably if we apply the
> coding standards unconditionally, rather than selectively.
>

That's right, but humanly enforced coding standard are usually a waste 
of time for everyone (reviewers and developers).

How many messages and exchanges on the mailing list could we save by 
using something like clang-format on the codebase, and force it to be 
"clean" as part of the CI? There would be no more discussion, as there 
would be only one single and objective source of truth.

> With regards,
> Daniel