Re: [PATCH] docs/style: allow C99 mixed declarations

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Feb 05, 2024 at 02:06:39PM -0500, Stefan Hajnoczi wrote:
> On Mon, 5 Feb 2024 at 13:16, Samuel Tardieu <sam@rfc1149.net> wrote:
> >
> >
> > Daniel P. Berrangé <berrange@redhat.com> writes:
> >
> > > $ gcc -Wall -Wuninitialized -o jump jump.c
> >
> > Note that many GCC warnings don't trigger if you don't enable
> > optimizations. In the case you exhibit, adding -O is enough to get
> > a sensible warning:
> >
> > $ gcc -Wall -O -o jump jump.c
> > jump.c: In function ‘main’:
> > jump.c:11:3: warning: ‘foo’ may be used uninitialized
> > [-Wmaybe-uninitialized]
> >    11 |   free(foo);
> >       |   ^~~~~~~~~
> > jump.c:8:9: note: ‘foo’ was declared here
> >     8 |   char *foo = malloc(30);
> >       |         ^~~
> 
> llvm also prints a warning:
> 
>   jump.c:5:7: warning: variable 'foo' is used uninitialized whenever
> 'if' condition is true [-Wsometimes-uninitialized]
> 
> I confirmed that QEMU's current compiler flags enable these warnings
> so both gcc and llvm detect the issue that Daniel pointed out in QEMU
> code.
> 
> Daniel: Does this address your concern about compiler warnings?

While it is good that its included when optimization is enabled, IME
GCC is not entirely reliable at detecting unintialized variables as
it has limits on extent of its flow control analysis.

Also consider the maint impact if a method includes C99 decls, and
a contributor later needs to add a 'goto' error cleanup path. They
might be forced to make a bunch of unrelated refactoring changes to
eliminate use of the C99 decls. This makes me not a fan of having a
situation where we tell contributors they can use C99 mixed decls,
except when they can't use them.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|