[patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[akpm]

From: Andrew Morton <akpm@linux-foundation.org>

drivers/atm/firestream.c: In function 'fs_open':
drivers/atm/firestream.c:870: warning: 'tmc0' may be used uninitialized in this function

I have confirmed that this is a false positive.

Cc: David S. Miller <davem@davemloft.net>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: chas williams <chas@cmf.nrl.navy.mil>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/atm/firestream.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/atm/firestream.c~drivers-atm-firestreamc-suppress-uninitialized-var-warning drivers/atm/firestream.c
--- a/drivers/atm/firestream.c~drivers-atm-firestreamc-suppress-uninitialized-var-warning
+++ a/drivers/atm/firestream.c
@@ -867,7 +867,7 @@ static int fs_open(struct atm_vcc *atm_v
 	int error;
 	int bfp;
 	int to;
-	unsigned short tmc0;
+	unsigned short uninitialized_var(tmc0);
 	short vpi = atm_vcc->vpi;
 	int vci = atm_vcc->vci;
 
_

Re: [patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[David Miller]

From: akpm@linux-foundation.org
Date: Fri, 28 Mar 2008 14:37:29 -0700

> From: Andrew Morton <akpm@linux-foundation.org>
> 
> drivers/atm/firestream.c: In function 'fs_open':
> drivers/atm/firestream.c:870: warning: 'tmc0' may be used uninitialized in this function
> 
> I have confirmed that this is a false positive.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jeff Garzik <jeff@garzik.org>
> Cc: chas williams <chas@cmf.nrl.navy.mil>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Andrew, I'm not applying patches that use that sinful
uninitialized_var() thing.

I'll apply the following instead.

commit d41a95e04ae80b77ddc186d0d97e6b439684adb8
Author: David S. Miller <davem@davemloft.net>
Date:   Fri Mar 28 16:19:26 2008 -0700

    [ATM] firestream: Fix uninitialized var warning.
    
    All code paths set tmc0 in some way, but GCC can't
    see that for some reason.  Explicitly initialize
    to zero.
    
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
index 47c57a4..98099f5 100644
--- a/drivers/atm/firestream.c
+++ b/drivers/atm/firestream.c
@@ -978,6 +978,7 @@ static int fs_open(struct atm_vcc *atm_vcc)
 		/* Docs are vague about this atm_hdr field. By the way, the FS
 		 * chip makes odd errors if lower bits are set.... -- REW */
 		tc->atm_hdr =  (vpi << 20) | (vci << 4); 
+		tmc0 = 0;
 		{
 			int pcr = atm_pcr_goal (txtp);
 

Re: [patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[Andrew Morton]

On Fri, 28 Mar 2008 16:20:17 -0700 (PDT) David Miller <davem@davemloft.net> wrote:

> From: akpm@linux-foundation.org
> Date: Fri, 28 Mar 2008 14:37:29 -0700
> 
> > From: Andrew Morton <akpm@linux-foundation.org>
> > 
> > drivers/atm/firestream.c: In function 'fs_open':
> > drivers/atm/firestream.c:870: warning: 'tmc0' may be used uninitialized in this function
> > 
> > I have confirmed that this is a false positive.
> > 
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Jeff Garzik <jeff@garzik.org>
> > Cc: chas williams <chas@cmf.nrl.navy.mil>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> 
> Andrew, I'm not applying patches that use that sinful
> uninitialized_var() thing.

Your call, but it isn't sinful.  It has documentary value - it tells the
reader "we don't really need to do this, but gcc isn't smart enough".

> I'll apply the following instead.
> 
> commit d41a95e04ae80b77ddc186d0d97e6b439684adb8
> Author: David S. Miller <davem@davemloft.net>
> Date:   Fri Mar 28 16:19:26 2008 -0700
> 
>     [ATM] firestream: Fix uninitialized var warning.
>     
>     All code paths set tmc0 in some way, but GCC can't
>     see that for some reason.  Explicitly initialize
>     to zero.
>     
>     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
> index 47c57a4..98099f5 100644
> --- a/drivers/atm/firestream.c
> +++ b/drivers/atm/firestream.c
> @@ -978,6 +978,7 @@ static int fs_open(struct atm_vcc *atm_vcc)
>  		/* Docs are vague about this atm_hdr field. By the way, the FS
>  		 * chip makes odd errors if lower bits are set.... -- REW */
>  		tc->atm_hdr =  (vpi << 20) | (vci << 4); 
> +		tmc0 = 0;
>  		{
>  			int pcr = atm_pcr_goal (txtp);

Whereas the reader doesn't know why that unneeded initialisation is there.

And it generates additional, unneeded code.

Re: [patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Fri, 28 Mar 2008 16:54:37 -0700

> > diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
> > index 47c57a4..98099f5 100644
> > --- a/drivers/atm/firestream.c
> > +++ b/drivers/atm/firestream.c
> > @@ -978,6 +978,7 @@ static int fs_open(struct atm_vcc *atm_vcc)
> >  		/* Docs are vague about this atm_hdr field. By the way, the FS
> >  		 * chip makes odd errors if lower bits are set.... -- REW */
> >  		tc->atm_hdr =  (vpi << 20) | (vci << 4); 
> > +		tmc0 = 0;
> >  		{
> >  			int pcr = atm_pcr_goal (txtp);
> 
> Whereas the reader doesn't know why that unneeded initialisation is there.

git show $(git blame drivers/atm/firestream.c | grep "tmc0 = 0" | \
	   awk ' { print $1 } ')

Re: [patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[Andrew Morton]

On Fri, 28 Mar 2008 16:58:12 -0700 (PDT) David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Fri, 28 Mar 2008 16:54:37 -0700
> 
> > > diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
> > > index 47c57a4..98099f5 100644
> > > --- a/drivers/atm/firestream.c
> > > +++ b/drivers/atm/firestream.c
> > > @@ -978,6 +978,7 @@ static int fs_open(struct atm_vcc *atm_vcc)
> > >  		/* Docs are vague about this atm_hdr field. By the way, the FS
> > >  		 * chip makes odd errors if lower bits are set.... -- REW */
> > >  		tc->atm_hdr =  (vpi << 20) | (vci << 4); 
> > > +		tmc0 = 0;
> > >  		{
> > >  			int pcr = atm_pcr_goal (txtp);
> > 
> > Whereas the reader doesn't know why that unneeded initialisation is there.
> 
> git show $(git blame drivers/atm/firestream.c | grep "tmc0 = 0" | \
> 	   awk ' { print $1 } ')

Not practical.  By this argument we wouldn't need code comments at all.

Plus the bit from my earlier email which you snipped.

Plus uninitialized_var() gives us something to grep for and experiment with
if/when future gcc's get smarter.

Re: [patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Fri, 28 Mar 2008 17:11:28 -0700

> Plus uninitialized_var() gives us something to grep for and
> experiment with if/when future gcc's get smarter.

That's sounds like a great investment of your time.

I guess there aren't any more interesting things to work
on, nor bugs to fix.

Re: [patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[Andrew Morton]

On Fri, 28 Mar 2008 17:35:23 -0700 (PDT) David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Fri, 28 Mar 2008 17:11:28 -0700
> 
> > Plus uninitialized_var() gives us something to grep for and
> > experiment with if/when future gcc's get smarter.
> 
> That's sounds like a great investment of your time.
> 
> I guess there aren't any more interesting things to work
> on, nor bugs to fix.

So you're all outa reasons.

I have explained our reasons for using uninitialized_var() rather than
sprinkling =0 all over the place, that's all.

Re: [patch 1/4] drivers/atm/firestream.c: suppress uninitialized var warning

[Al Viro]

On Fri, Mar 28, 2008 at 05:39:46PM -0700, Andrew Morton wrote:
> On Fri, 28 Mar 2008 17:35:23 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
> 
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Date: Fri, 28 Mar 2008 17:11:28 -0700
> > 
> > > Plus uninitialized_var() gives us something to grep for and
> > > experiment with if/when future gcc's get smarter.
> > 
> > That's sounds like a great investment of your time.
> > 
> > I guess there aren't any more interesting things to work
> > on, nor bugs to fix.
> 
> So you're all outa reasons.

Not really...  "we know better, gcc must STFU" applies _now_.  And gcc getting
smarter is not the only way it can become false - code change in the function
can create real uninitialized use.  gcc tends to change the warning on that,
but not if it's told to STFU in either way.

IOW, my preference would be to leave the warning as is.