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

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

[hbuxiaofei]

  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

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);
-- 
2.27.0

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

[hbuxiaofei]

  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

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);
-- 
2.27.0

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

[Andre Przywara]

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);

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

[Will Deacon]

On Wed, 2 Nov 2022 16:05:01 +0800, hbuxiaofei wrote:
>   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
> 
> [...]

Applied to kvmtool (master), thanks!

[1/1] hw/i8042: Fix value uninitialized in kbd_io()
      https://git.kernel.org/will/kvmtool/c/5a9cde6532ea

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

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

[hbuxiaofei]

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