Re: [PATCH kvmtool] hw/i8042: Fix value uninitialized in kbd_io()

[hbuxiaofei <hbuxiaofei@gmail.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB18030, Size: 1920 bytes --]

On Wed, 9 Nov 2022 10:52:53 +0800
Xiaofei <hbuxiaofei@gmail.com> wrote:

> On Wed,  2 Nov 2022 16:05:01 +0800
> hbuxiaofei <hbuxiaofei@gmail.com> wrote:
> 
> Hi,
> 
> >   GCC Version:
> >     gcc (GCC) 8.4.1 20200928 (Red Hat 8.4.1-1)
> >
> >   hw/i8042.c: In function ¡®kbd_io¡¯:
> >   hw/i8042.c:153:19: error: ¡®value¡¯ may be used uninitialized in
> > this  
> function [-Werror=maybe-uninitialized]
> >      state.write_cmd = val;
> >      ~~~~~~~~~~~~~~~~^~~~~
> >   hw/i8042.c:298:5: note: ¡®value¡¯ was declared here
> >     u8 value;
> >        ^~~~~
> >   cc1: all warnings being treated as errors
> >   make: *** [Makefile:508: hw/i8042.o] Error 1  
> 
> Yeah, I have seen this with the Ubuntu 18.04 GCC as well (Ubuntu
> 7.5.0-3ubuntu1-18.04), when compiling for x86. It's pretty clearly a
> compiler bug (or rather inability to see through all the branches),
> but as the code currently stands, value will always be initialised.
> So while it's easy to brush this off as "go and fix your compiler",
> for users of Ubuntu 18.04 and RedHat 8 that's probably not an easy
> thing to do. So since we force breakage on people by using Werror,
> I'd support the idea of taking this patch, potentially with a
> comment, to make people's life easier.
> 
> Cheers,
> Andre
> 
> > Signed-off-by: hbuxiaofei <hbuxiaofei@gmail.com>
> > ---
> >  hw/i8042.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/i8042.c b/hw/i8042.c
> > index 20be36c..6e4b559 100644
> > --- a/hw/i8042.c
> > +++ b/hw/i8042.c
> > @@ -295,7 +295,7 @@ static void kbd_reset(void)
> >  static void kbd_io(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32
> > len, u8 is_write, void *ptr)
> >  {
> > -     u8 value;
> > +     u8 value = 0;
> >
> >       if (is_write)
> >               value = ioport__read8(data);  

Thanks for your advice, it is indeed a compiler bug. Let me add some
comments.

Best regards
Xiaofei