[PATCH] vt: keyboard, fix uninitialized variables warning

[PATCH] vt: keyboard, fix uninitialized variables warning

[Li Wang]

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@windriver.com>
---
 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;
-- 
2.7.4

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

[Greg KH]

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@windriver.com>
> ---
>  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...

thanks,

greg k-h

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

[Wang, Li]

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@windriver.com>
>> ---
>>   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.

thank for your reply,
LiWang.

>
> thanks,
>
> greg k-h

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

[Greg KH]

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@windriver.com>
> > > ---
> > >   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 :(

thanks,

greg k-h

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

[Andy Shevchenko]

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@windriver.com>
> > > > ---
> > > >   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.

-- 
With Best Regards,
Andy Shevchenko

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@windriver.com>
>>>>> ---
>>>>>    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@linux-foundation.org>
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@linux-foundation.org>

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

[Li Wang]

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;
         ^~~
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@windriver.com>
---
 drivers/tty/vt/keyboard.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 7763862..62f1ecb 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -2090,6 +2090,8 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 
 		ret = 0;
 		break;
+	default:
+		return -EINVAL;
 	}
 
 	kfree(kbs);
-- 
2.7.4

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

[Jiri Slaby]

On 04. 03. 21, 4:10, 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;
>           ^~~
> 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@windriver.com>
> ---
>   drivers/tty/vt/keyboard.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
> index 7763862..62f1ecb 100644
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -2090,6 +2090,8 @@ int vt_do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
>   
>   		ret = 0;
>   		break;
> +	default:
> +		return -EINVAL;

I am not biased whether to add it or not, but I would return 
-ENOIOCTLCMD if we do.

>   	}
>   
>   	kfree(kbs);
> 


-- 
js
suse labs

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

[Greg KH]

On Thu, Mar 04, 2021 at 11:10:48AM +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;
>          ^~~
> drivers/tty/vt/keyboard.c: warning: 'kbs' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   kfree(kbs);
>   ^~~~~~~~~~

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

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

[Andy Shevchenko]

On Thu, Mar 04, 2021 at 11:10:48AM +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;
>          ^~~
> drivers/tty/vt/keyboard.c: warning: 'kbs' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   kfree(kbs);


Let me add one more comment and summarize altogether:
 - Jiri wants you to have different error code
 - Greg (and I noticed that as well) wants you to add a proper commit message,
   and not just some output of some tool w/o context
 - I want you to send a new version w/o chaining to the previous thread, so
   start new (email) thread every time you send a new version

Waiting for v3 in a separate email thread, thanks!

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Thu, Mar 04, 2021 at 02:28:55PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 04, 2021 at 11:10:48AM +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;
> >          ^~~
> > drivers/tty/vt/keyboard.c: warning: 'kbs' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >   kfree(kbs);
> 
> 
> Let me add one more comment and summarize altogether:
>  - Jiri wants you to have different error code
>  - Greg (and I noticed that as well) wants you to add a proper commit message,
>    and not just some output of some tool w/o context
>  - I want you to send a new version w/o chaining to the previous thread, so
>    start new (email) thread every time you send a new version
> 
> Waiting for v3 in a separate email thread, thanks!

Last, but not least: add a changelog after the cutter '---' line, so people will know what have you done on the transition from vX to vX+1.

-- 
With Best Regards,
Andy Shevchenko