fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Toralf Förster]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]

I compiled current git source 2.6.21-rc5-g28defbe and got this warning:
...
fs/block_dev.c: In function `bd_claim_by_kobject':
fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function
...

-- 
MfG/Sincerely

Toralf Förster

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Lee Revell]

On 3/28/07, Toralf Förster <toralf.foerster@gmx.de> wrote:
> I compiled current git source 2.6.21-rc5-g28defbe and got this warning:
> ...
> fs/block_dev.c: In function `bd_claim_by_kobject':
> fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function
> ...
>

Most of these warnings are really GCC bugs.  Please examine the code
in question.

Lee

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Jiri Kosina]

On Wed, 28 Mar 2007, Lee Revell wrote:

> > I compiled current git source 2.6.21-rc5-g28defbe and got this warning:
> > ...
> > fs/block_dev.c: In function `bd_claim_by_kobject':
> > fs/block_dev.c:953: warning: 'found' might be used uninitialized in this
> > function
> > ...
> Most of these warnings are really GCC bugs.  Please examine the code
> in question.

Anyway imho this time gcc got it right?



From: Jiri Kosina <jkosina@suse.cz>

blockdev: bd_claim_by_kobject() could check value of unititalized pointer

Fixes this warning:

fs/block_dev.c: In function `bd_claim_by_kobject':
fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

struct bd_holder *found is initialized only when bd_claim() returns zero. If it
returns nonzero, ptr stays uninitialized. Later the value of the pointer is checked.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>

diff --git a/drivers/usb/input/hid-tmff.c b/drivers/usb/input/hid-tmff.c
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 575076c..e87d84a 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -950,7 +950,7 @@ static int bd_claim_by_kobject(struct block_device *bdev, void *holder,
 				struct kobject *kobj)
 {
 	int res;
-	struct bd_holder *bo, *found;
+	struct bd_holder *bo, *found = NULL;
 
 	if (!kobj)
 		return -EINVAL;

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Andrew Morton]

On Wed, 28 Mar 2007 19:23:32 +0200 (CEST)
Jiri Kosina <jikos@jikos.cz> wrote:

> blockdev: bd_claim_by_kobject() could check value of unititalized pointer
> 
> Fixes this warning:
> 
> fs/block_dev.c: In function `bd_claim_by_kobject':
> fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function
> 
> struct bd_holder *found is initialized only when bd_claim() returns zero. If it
> returns nonzero, ptr stays uninitialized. Later the value of the pointer is checked.
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> 
> diff --git a/drivers/usb/input/hid-tmff.c b/drivers/usb/input/hid-tmff.c
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 575076c..e87d84a 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -950,7 +950,7 @@ static int bd_claim_by_kobject(struct block_device *bdev, void *holder,
>  				struct kobject *kobj)
>  {
>  	int res;
> -	struct bd_holder *bo, *found;
> +	struct bd_holder *bo, *found = NULL;

that generates extra code and people get upset.

One approach which we could ue in here is

	struct bd_holder *found = found;  /* Suppress bogus gcc warning */

which (surprisingly) gcc will currently accept, and which shouldn't
generate more code, although I haven't verified this.


But it's all rather ad-hoc and unpleasant.  I tend to think that we should
come up with some standardised way of squashing this warning - something
which stands out when one is reading the code, like

	struct bd_holder *found;

	squash_bogus_uninit_warning(found);	/* useful comment goes here */

which is also unpleasant, but not as unpleasant as a screenful of warnings
which hide real problems, IMO.

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Dan Aloni]

On Wed, Mar 28, 2007 at 01:14:54PM -0700, Andrew Morton wrote:
> On Wed, 28 Mar 2007 19:23:32 +0200 (CEST)
> Jiri Kosina <jikos@jikos.cz> wrote:
> 
> > blockdev: bd_claim_by_kobject() could check value of unititalized pointer
[..]
> > @@ -950,7 +950,7 @@ static int bd_claim_by_kobject(struct block_device *bdev, void *holder,
> >  				struct kobject *kobj)
> >  {
> >  	int res;
> > -	struct bd_holder *bo, *found;
> > +	struct bd_holder *bo, *found = NULL;
> 
> that generates extra code and people get upset.

I, for one, not upset. On the contrary.

IMHO gcc should be smart enough to optimize that code properly with that 
"= NULL" added. 

BTW with gcc 4.1.2 on x86_64 that warning doesn't get emitted, and it 
generates the same exact code with or without " = NULL". One could aruge,
if people are upset about more code being generated because they use an 
older stable branch of gcc, it's _their_ problem.

> 	struct bd_holder *found;
> 
> 	squash_bogus_uninit_warning(found);	/* useful comment goes here */
> 
> which is also unpleasant, but not as unpleasant as a screenful of warnings
> which hide real problems, IMO.

If there was such 'squash_bogus_uninit_warning' macro exist and in use,
then this could have been a possible scenario:

  A) There's some 200-lines long function.
  B) It has a squash_bogus_uninit_warning() somewhere in the beginning.
  C) Someone commits a patch that uses an uninitialized variable on _some_
     cases and it doesn't generate a warning.
  D) You get an 'heisenbug', since that pointer might point to something
     that is dereferencable without a fault, etc.

I think that warnings of these kind (assuming that they are not generated
as a result of deficiencies in the latest stable version of gcc) exist
for a damn good reason - the code should be fixed and that warning 
shouldn't be bypassed in semi-nasty ways.

-- 
Dan Aloni
XIV LTD, http://www.xivstorage.com
da-x (at) monatomic.org, dan (at) xiv.co.il

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Kyle Moffett]

On Mar 28, 2007, at 16:14:54, Andrew Morton wrote:
> On Wed, 28 Mar 2007 19:23:32 +0200 (CEST)
> Jiri Kosina <jikos@jikos.cz> wrote:
>
>> blockdev: bd_claim_by_kobject() could check value of unititalized  
>> pointer
>>
>> Fixes this warning:
>>
>> fs/block_dev.c: In function `bd_claim_by_kobject':
>> fs/block_dev.c:953: warning: 'found' might be used uninitialized  
>> in this function
>>
>> struct bd_holder *found is initialized only when bd_claim()  
>> returns zero. If it returns nonzero, ptr stays uninitialized.  
>> Later the value of the pointer is checked.
>
> that generates extra code and people get upset.
>
> One approach which we could ue in here is
>
> 	struct bd_holder *found = found;  /* Suppress bogus gcc warning */

Well, that would be correct except the warning is an actual kernel  
bug.  Read Jiri's message (which you also quoted):
> struct bd_holder *found is initialized only when bd_claim() returns  
> zero. If it returns nonzero, ptr stays uninitialized. Later the  
> value of the pointer is checked.

So in this case it has to be initialized to NULL or there's a  
potential BUG() lurking.

Cheers,
Kyle Moffett

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Adrian Bunk]

On Thu, Mar 29, 2007 at 11:16:39PM -0400, Kyle Moffett wrote:
> On Mar 28, 2007, at 16:14:54, Andrew Morton wrote:
> >On Wed, 28 Mar 2007 19:23:32 +0200 (CEST)
> >Jiri Kosina <jikos@jikos.cz> wrote:
> >
> >>blockdev: bd_claim_by_kobject() could check value of unititalized  
> >>pointer
> >>
> >>Fixes this warning:
> >>
> >>fs/block_dev.c: In function `bd_claim_by_kobject':
> >>fs/block_dev.c:953: warning: 'found' might be used uninitialized  
> >>in this function
> >>
> >>struct bd_holder *found is initialized only when bd_claim()  
> >>returns zero. If it returns nonzero, ptr stays uninitialized.  
> >>Later the value of the pointer is checked.
> >
> >that generates extra code and people get upset.
> >
> >One approach which we could ue in here is
> >
> >	struct bd_holder *found = found;  /* Suppress bogus gcc warning */
> 
> Well, that would be correct except the warning is an actual kernel  
> bug.  Read Jiri's message (which you also quoted):
> >struct bd_holder *found is initialized only when bd_claim() returns  
> >zero. If it returns nonzero, ptr stays uninitialized. Later the  
> >value of the pointer is checked.
> 
> So in this case it has to be initialized to NULL or there's a  
> potential BUG() lurking.


No, the code is correct and it's impossible that the variable ever gets 
read uninitialized.

And BTW, i386 gcc 4.1 doesn't give me a warning for this.
Toralf, which gcc version and architecture did you see this with?


> Cheers,
> Kyle Moffett

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Cong WANG]

2007/3/31, Adrian Bunk <bunk@stusta.de>:
> On Thu, Mar 29, 2007 at 11:16:39PM -0400, Kyle Moffett wrote:
> > On Mar 28, 2007, at 16:14:54, Andrew Morton wrote:
> > >On Wed, 28 Mar 2007 19:23:32 +0200 (CEST)
> > >Jiri Kosina <jikos@jikos.cz> wrote:
> > >
> > >>blockdev: bd_claim_by_kobject() could check value of unititalized
> > >>pointer
> > >>
> > >>Fixes this warning:
> > >>
> > >>fs/block_dev.c: In function `bd_claim_by_kobject':
> > >>fs/block_dev.c:953: warning: 'found' might be used uninitialized
> > >>in this function
> > >>
> > >>struct bd_holder *found is initialized only when bd_claim()
> > >>returns zero. If it returns nonzero, ptr stays uninitialized.
> > >>Later the value of the pointer is checked.
> > >
> > >that generates extra code and people get upset.
> > >
> > >One approach which we could ue in here is
> > >
> > >     struct bd_holder *found = found;  /* Suppress bogus gcc warning */
> >
> > Well, that would be correct except the warning is an actual kernel
> > bug.  Read Jiri's message (which you also quoted):
> > >struct bd_holder *found is initialized only when bd_claim() returns
> > >zero. If it returns nonzero, ptr stays uninitialized. Later the
> > >value of the pointer is checked.
> >
> > So in this case it has to be initialized to NULL or there's a
> > potential BUG() lurking.
>
>
> No, the code is correct and it's impossible that the variable ever gets
> read uninitialized.
>
> And BTW, i386 gcc 4.1 doesn't give me a warning for this.
> Toralf, which gcc version and architecture did you see this with?
>

I am also using i386 gcc 4.1.1, and I did receive many warnings of
such kind yesterday. I think we should fix them.

And the reason for the existence of such things is we just want to use
them for writing first instead of reading, thus ignore the
initialization.

-- 
So Dark The Con Of Man.

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Toralf Förster]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

Am Freitag, 30. März 2007 21:47 schrieb Adrian Bunk:
> 
> 
> No, the code is correct and it's impossible that the variable ever gets 
> read uninitialized.
> 
> And BTW, i386 gcc 4.1 doesn't give me a warning for this.
> Toralf, which gcc version and architecture did you see this with?

> cu
> Adrian
> 

Hi, I use currently the following version:

gcc version 3.4.6 (Gentoo 3.4.6-r2, ssp-3.4.6-1.0, pie-8.7.10)

-- 
MfG/Sincerely

Toralf Förster

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Adrian Bunk]

On Sat, Mar 31, 2007 at 10:11:27AM +0200, Toralf Förster wrote:
> Am Freitag, 30. März 2007 21:47 schrieb Adrian Bunk:
> > 
> > 
> > No, the code is correct and it's impossible that the variable ever gets 
> > read uninitialized.
> > 
> > And BTW, i386 gcc 4.1 doesn't give me a warning for this.
> > Toralf, which gcc version and architecture did you see this with?
> 
> > cu
> > Adrian
> > 
> 
> Hi, I use currently the following version:
> 
> gcc version 3.4.6 (Gentoo 3.4.6-r2, ssp-3.4.6-1.0, pie-8.7.10)

For finding warnings it's better to use the latest gcc version - gcc is 
evalving, and more recent versions tend to both give more valid warnings 
and give less warnings for non-problems (like in this case).

> MfG/Sincerely
> 
> Toralf Förster

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: fs/block_dev.c:953: warning: 'found' might be used uninitialized in this function

[Adrian Bunk]

On Wed, Mar 28, 2007 at 12:56:59PM -0400, Lee Revell wrote:
> On 3/28/07, Toralf Förster <toralf.foerster@gmx.de> wrote:
> >I compiled current git source 2.6.21-rc5-g28defbe and got this warning:
> >...
> >fs/block_dev.c: In function `bd_claim_by_kobject':
> >fs/block_dev.c:953: warning: 'found' might be used uninitialized in this 
> >function
> >...
> >
> 
> Most of these warnings are really GCC bugs.  Please examine the code
> in question.

The word "GCC bugs" is a bit hard for them.

gcc gives different warnings for code where it could prove that a 
variable gets used uninitialized and code where it's not sure whether a 
variable will always be used initialized.

There are even cases where it's technically impossible for gcc to 
determine that a variable always gets initialized.

And these "might be used uninitialized" warnings have already found 
several kernel bugs.

> Lee

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed