[PATCH] qemu: tpm: Initialize variable with NULL

[Daniel P. Berrangé]

On Mon, Nov 08, 2021 at 06:06:50PM +0400, Marc-André Lureau wrote:
> On Mon, Nov 8, 2021 at 5:53 PM Stefan Berger <stefanb at linux.ibm.com> wrote:
> >
> > Initialize an autofree'd variable with NULL that causes crashes
> > if a TPM 1.2 is used and the variable doesn't get a value assigned.
> >
> > Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
> 
> certainly, I wished there would be a gcc error there..

I'd support a coding rule for libvirt that *every* single stack variable
must *always* be initialized at time of declaration, even if there's an
initialization happening later in the method, even if it is on the very
next line.

We've had way too many bugs from leaving variables uninitialized over
the years, that mandatory explicit init is worthwhile on balance IMHO.

It isn't that easy to detect this in a coding style rule though, as
parsing C with regexes is flakey at best. I wonder if libclang could
be used to write a better style check.

Meanwhile, I'm looking forward to GCC 12 which introduces a flag

   $CC -ftrivial-auto-var-init=zero

to tell the compiler to initialize everything to zero. Annoyingly
that LLVM maintainers have had this in clang for a while, but don't
want to officially support this nice enhancement to the security
and reliability of C code, so hide this feature behind a crazy
flag [1]

> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> > ---
> >  src/qemu/qemu_tpm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> > index b305313ad2..1992b596cb 100644
> > --- a/src/qemu/qemu_tpm.c
> > +++ b/src/qemu/qemu_tpm.c
> > @@ -610,7 +610,7 @@ qemuTPMEmulatorReconfigure(const char *storagepath,
> >  {
> >      g_autoptr(virCommand) cmd = NULL;
> >      int exitstatus;
> > -    g_autofree char *activePcrBanksStr;
> > +    g_autofree char *activePcrBanksStr = NULL;
> >      g_autofree char *swtpm_setup = virTPMGetSwtpmSetup();
> >      VIR_AUTOCLOSE pwdfile_fd = -1;
> >
> > --
> > 2.31.1
> >
> 

Regards,
Daniel

[1]  $  clang -ftrivial-auto-var-init=zero demo.c
     clang-12: error: -ftrivial-auto-var-init=zero hasn't been enabled. Enable it at your own peril for benchmarking purpose only with -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang

    
-- 
|: 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 :|