Re: [PATCH] Change the default for Mixed declarations.

["Alex Bennée" <alex.bennee@linaro.org>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Feb 14, 2023 at 05:07:38PM +0100, Juan Quintela wrote:
>> Hi
>> 
>> I want to enter a discussion about changing the default of the style
>> guide.
>> 
>> There are several reasons for that:
>> - they exist since C99 (i.e. all supported compilers support them)
>> - they eliminate the posibility of an unitialized variable.
>
> Actually they don't do that reliably. In fact, when combined
> with usage of 'goto', they introduce uninitialized variables,
> despite the declaration having an initialization present, and
> thus actively mislead reviewers into thinking their code is
> safe.
>
<snip>
>
>> - (at least for me), declaring the index inside the for make clear
>>   that index is not used outside the for.
>
> I'll admit that declaring loop indexes in the for() is a nice
> bit, but I'm not a fan in general of mixing the declarations
> in the middle of code for projects that use the 'goto cleanup'
> pattern.

I think we could just finesse the rules to allow declaring within the
for() as allowable as start of block. I think more freedom to declare on
first use is only warranted when the compiler will stop you foot gunning
yourself (as it does in Rust).

>> - Current documentation already declares that they are allowed in some
>>   cases.
>> - Lots of places already use them.
>> 
>> We can change the text to whatever you want, just wondering if it is
>> valib to change the standard.
>> 
>> Doing a trivial grep through my local qemu messages (around 100k) it
>> shows that some people are complaining that they are not allowed, and
>> other saying that they are used all over the place.
>
> IMHO the status quo is bad because it is actively dangerous when
> combined with goto and we aren't using any compiler warnings to
> help us.
>
> Either we allow it, but use -Wjump-misses-init to prevent mixing
> delayed declarations with gotos, and just avoid this when it triggers
> a false positive.

Has anyone looked to see how much this triggers on the code base as is?

> Or we forbid it, rewrite current cases that use it, and then add
> -Wdeclaration-after-statement to enforce it.
>
>
> IMHO if we are concerned about uninitialized variables then I think
> a better approach is to add -ftrivial-auto-var-init=zero, which will
> make the compiler initialize all variables to 0 if they lack an
> explicit initializer.

Would that make us less likely to find the occasional bug that does fire
when missing inititizers could be random?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro