Re: [PATCH] vt: keyboard, fix uninitialized variables warning

[Wang, Li]

On 3/3/2021 4:24 PM, Andy Shevchenko wrote:
> On Wed, Mar 03, 2021 at 08:39:09AM +0100, Greg KH wrote:
> > On Wed, Mar 03, 2021 at 03:33:23PM +0800, Wang, Li wrote:
> > > On 3/3/2021 3:14 PM, Greg KH wrote:
> > > > On Wed, Mar 03, 2021 at 12:59:32PM +0800, Li Wang wrote:
> > > > > drivers/tty/vt/keyboard.c: In function 'vt_do_kdgkb_ioctl':
> > > > > drivers/tty/vt/keyboard.c: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > > >     return ret;
> > > > >            ^~~
> > > > > kernel-source/drivers/tty/vt/keyboard.c: warning: 'kbs' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > > >     kfree(kbs);
> > > > >     ^~~~~~~~~~
> > > > >
> > > > > Signed-off-by: Li Wang <li.wang@xxxxxxxxxxxxx>
> > > > > ---
> > > > >    drivers/tty/vt/keyboard.c | 4 ++--
> > > > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
> > > > > index 7763862..3e73d55 100644
> > > > > --- a/drivers/tty/vt/keyboard.c
> > > > > +++ b/drivers/tty/vt/keyboard.c
> > > > > @@ -2049,8 +2049,8 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
> > > > >    {
> > > > >    	unsigned char kb_func;
> > > > >    	unsigned long flags;
> > > > > -	char *kbs;
> > > > > -	int ret;
> > > > > +	char *kbs = NULL;
> > > > > +	int ret = -EINVAL;
> > > > >    	if (get_user(kb_func, &user_kdgkb->kb_func))
> > > > >    		return -EFAULT;
> > > > What compiler is providing these "warnings"?
> > > >
> > > > Turns out it is impossible to hit, so this isn't actually fixing
> > > > anything...
> > > I tested it with gcc 8.2 for arm
> > >
> > > for runtime codes view, indeed it is impossible to hit.
> > >
> > > but for compiler view, gcc should give 'used uninitialized' warning, too.
> > Odd that no other compiler version does this right now, perhaps upgrade
> > to a newer version of gcc?  8.2 is really old :(
> But it's still supported. I think I can see why. We have a switch case without
> default, and probably that's how it makes that happen. So, the proper fix is to
> add default section AFAICT.

Hi all,

I tried the latest gcc and linux, these warnings disappear.
it is not gcc issue, the issue is that I do not use the latest linux.

linux disables the 'used uninitialized' warning in the below commit,
but mentioned by description, if want to debug kernel warning, use 
option to open.
and 'great ... code ... never confuses the compiler'

I will according to Andy's suggest, send V2.
Thanks,
LiWang.

=====

From 78a5255ffb6a1af189a83e493d916ba1c54d8c75 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Sat, 9 May 2020 13:57:10 -0700
Subject: [PATCH] Stop the ad-hoc games with -Wno-maybe-initialized

We have some rather random rules about when we accept the
"maybe-initialized" warnings, and when we don't.

For example, we consider it unreliable for gcc versions < 4.9, but also
if -O3 is enabled, or if optimizing for size.  And then various kernel
config options disabled it, because they know that they trigger that
warning by confusing gcc sufficiently (ie PROFILE_ALL_BRANCHES).

And now gcc-10 seems to be introducing a lot of those warnings too, so
it falls under the same heading as 4.9 did.

At the same time, we have a very straightforward way to _enable_ that
warning when wanted: use "W=2" to enable more warnings.

So stop playing these ad-hoc games, and just disable that warning by
default, with the known and straight-forward "if you want to work on the
extra compiler warnings, use W=123".

Would it be great to have code that is always so obvious that it never
confuses the compiler whether a variable is used initialized or not?
Yes, it would.  In a perfect world, the compilers would be smarter, and
our source code would be simpler.

That's currently not the world we live in, though.

Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>