[PATCH -mm] sys_semctl gcc 4.1 warning fix

[PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

Fixes the following warnings,

ipc/sem.c: In function 'sys_semctl':
ipc/sem.c:810: warning: 'setbuf.uid' may be used uninitialized in this function
ipc/sem.c:810: warning: 'setbuf.gid' may be used uninitialized in this function
ipc/sem.c:810: warning: 'setbuf.mode' may be used uninitialized in this function

Signed-Off-By: Daniel Walker <dwalker@mvista.com>

Index: linux-2.6.16/ipc/sem.c
===================================================================
--- linux-2.6.16.orig/ipc/sem.c
+++ linux-2.6.16/ipc/sem.c
@@ -807,7 +807,7 @@ static int semctl_down(int semid, int se
 {
 	struct sem_array *sma;
 	int err;
-	struct sem_setbuf setbuf;
+	struct sem_setbuf setbuf = {0, 0, 0};
 	struct kern_ipc_perm *ipcp;
 
 	if(cmd == IPC_SET) {

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Alan Cox]

On Maw, 2006-05-09 at 19:56 -0700, Daniel Walker wrote:
> Fixes the following warnings,
> 
> ipc/sem.c: In function 'sys_semctl':
> ipc/sem.c:810: warning: 'setbuf.uid' may be used uninitialized in this function
> ipc/sem.c:810: warning: 'setbuf.gid' may be used uninitialized in this function
> ipc/sem.c:810: warning: 'setbuf.mode' may be used uninitialized in this function
> 
> Signed-Off-By: Daniel Walker <dwalker@mvista.com>
> 
> Index: linux-2.6.16/ipc/sem.c
> ===================================================================
> --- linux-2.6.16.orig/ipc/sem.c
> +++ linux-2.6.16/ipc/sem.c
> @@ -807,7 +807,7 @@ static int semctl_down(int semid, int se
>  {
>  	struct sem_array *sma;
>  	int err;
> -	struct sem_setbuf setbuf;
> +	struct sem_setbuf setbuf = {0, 0, 0};


This causes very poor code as its initializing an object on the stack.
It also appears from inspection to be entirely un-neccessary. Instead
the compiler needs some help.

Hiding warnings like this can be a hazard as it will hide real warnings
later on.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 11:34 +0100, Alan Cox wrote:
> 
> This causes very poor code as its initializing an object on the stack.
> It also appears from inspection to be entirely un-neccessary. Instead
> the compiler needs some help.

It's just a warning .. The compiler flagged a spot which look suspect .
We have -Wall turn on now , so we could possibly disable these
warnings. 

> Hiding warnings like this can be a hazard as it will hide real warnings
> later on.

How could it hide real warnings? If anything these patch allow other
(real warnings) to be seen . 

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Alan Cox]

On Mer, 2006-05-10 at 07:31 -0700, Daniel Walker wrote:
> > Hiding warnings like this can be a hazard as it will hide real warnings
> > later on.
> 
> How could it hide real warnings? If anything these patch allow other
> (real warnings) to be seen . 

Because while the warning is present people will check it now and again.
If you set the variable to zero then you generate extra code and you
ensure nobody will look in future.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 16:09 +0100, Alan Cox wrote:
> On Mer, 2006-05-10 at 07:31 -0700, Daniel Walker wrote:
> > > Hiding warnings like this can be a hazard as it will hide real warnings
> > > later on.
> > 
> > How could it hide real warnings? If anything these patch allow other
> > (real warnings) to be seen . 
> 
> Because while the warning is present people will check it now and again.

But it's pointless to review it, in this case and for this warning .

> If you set the variable to zero then you generate extra code and you
> ensure nobody will look in future.

The extra code is a problem , I'll admit that . But the warning should
disappear , it's just a waste .

Daniel

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

On Wed, 10 May 2006, Daniel Walker wrote:

> On Wed, 2006-05-10 at 16:09 +0100, Alan Cox wrote:
> > On Mer, 2006-05-10 at 07:31 -0700, Daniel Walker wrote:
> > > > Hiding warnings like this can be a hazard as it will hide real warnings
> > > > later on.
> > >
> > > How could it hide real warnings? If anything these patch allow other
> > > (real warnings) to be seen .
> >
> > Because while the warning is present people will check it now and again.
>
> But it's pointless to review it, in this case and for this warning .
>
> > If you set the variable to zero then you generate extra code and you
> > ensure nobody will look in future.
>
> The extra code is a problem , I'll admit that . But the warning should
> disappear , it's just a waste .
>

What is really needed is an attribute to add to function parameters, that
tells gcc that the parameter, if a pointer, will be initialize via the
function.

For example:

#define __assigned  __attribute__((initialized))

then declare a function:

int my_init_variabl(__assigned struct mystruct *var);

So gcc can know that the passed in variable will be updated. It could
then check to see if the function actually does initialize the pointer,
if the declaration is visible to the function definition itself.

Any gcc developers watching :)

-- Steve

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Adrian Bunk]

On Wed, May 10, 2006 at 11:24:31AM -0400, Steven Rostedt wrote:
> 
> On Wed, 10 May 2006, Daniel Walker wrote:
> 
> > On Wed, 2006-05-10 at 16:09 +0100, Alan Cox wrote:
> > > On Mer, 2006-05-10 at 07:31 -0700, Daniel Walker wrote:
> > > > > Hiding warnings like this can be a hazard as it will hide real warnings
> > > > > later on.
> > > >
> > > > How could it hide real warnings? If anything these patch allow other
> > > > (real warnings) to be seen .
> > >
> > > Because while the warning is present people will check it now and again.
> >
> > But it's pointless to review it, in this case and for this warning .
> >
> > > If you set the variable to zero then you generate extra code and you
> > > ensure nobody will look in future.
> >
> > The extra code is a problem , I'll admit that . But the warning should
> > disappear , it's just a waste .
> >
> 
> What is really needed is an attribute to add to function parameters, that
> tells gcc that the parameter, if a pointer, will be initialize via the
> function.
> 
> For example:
> 
> #define __assigned  __attribute__((initialized))
> 
> then declare a function:
> 
> int my_init_variabl(__assigned struct mystruct *var);
> 
> So gcc can know that the passed in variable will be updated. It could
> then check to see if the function actually does initialize the pointer,
> if the declaration is visible to the function definition itself.
> 
> Any gcc developers watching :)


It seems you don't understand the problem at all:


First of all note that your example does not apply in this case:

copy_semid_from_user() does _not_ initialize sembuf in all cases.


And you do not understand where gcc's problem is:

gcc does in fact see that setbuf is always initialized if 
copy_semid_from_user() returns 0.

setbuf is only initialized in the (cmd == IPC_SET) case and later only 
used in the (cmd == IPC_SET) case. And cmd can't change between the two 
occurences.

Therefore, gcc should in theory already have enough information to prove 
that sembuf is always initialized before it's used.


> -- Steve


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: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

On Wed, 10 May 2006, Adrian Bunk wrote:

>
> It seems you don't understand the problem at all:
>

Actually I do, altough I admit my reply was for more the general case with
pointers. And that I was using TAGS to jump around the function and didn't
notice that copy_semid_from_user was defined just above the function :-/
OK, it's getting late for me.

>
> First of all note that your example does not apply in this case:
>
> copy_semid_from_user() does _not_ initialize sembuf in all cases.

Yes, but the attribute could still remove the warning without adding more
instructions.  And the fact that the attribute is there, we can use it to
know that it was looked at and remind us that it's used.

OK, we could just have an attribute on the declaration of the variable
that keeps that warning turned off.  It also lets us know that someone
looked at that variable to make sure it is ok. And we can still undefine
the attribute definition with a make parameter if we want to recheck all
those warnings.

>
> And you do not understand where gcc's problem is:
>
> gcc does in fact see that setbuf is always initialized if
> copy_semid_from_user() returns 0.


>
> setbuf is only initialized in the (cmd == IPC_SET) case and later only
> used in the (cmd == IPC_SET) case. And cmd can't change between the two
> occurences.
>
> Therefore, gcc should in theory already have enough information to prove
> that sembuf is always initialized before it's used.
>

OK in theory, with this example it does have enough info.  But how much
info do you want gcc to hold while it's compiling?

To know that it's ok you need to know the following:

(what you've mentioned)
 setbuf is initialized
     cmd == IPC_SET
      and
     if copy_sem_from_user returns zero.

 setbuf isn't used unless cmd == IPC_SET

Now gcc will also need to know (which is there, but extra info for
compiling):

  setbuf->uid is updated in copy_sem_from_user and it returns zero.
  setbuf->out is updated in copy_sem_from_user and it returns zero.
  setbuf->mode is updated in copy_sem_from_user and it returns zero.

So it isn't enough to just know sembuf was updated, but you also need to
know what parts of the structure is initialized and used.  This example is
trivial, but what happens when you have structures with 100 elements, and
some with pointers to other elements.  Should gcc keep track of all that
too?

-- Steve

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

Oh fsck! gcc is hosed. I just tried out this BS module:

---
#include <linux/module.h>

int g = 0;

void func_int(int *y)
{
	*y = 0;
}

int warn_here(void)
{
	int x;

	printk("x=%d\n",x);

	return 0;
}

int but_not_here(void)
{
	int y;

	printk("y=%d\n",y);
	if (g) {
		func_int(&y);
	}
	return 0;
}

static int __init blah_init(void)
{
	warn_here();
	but_not_here();
        return 0;
}

static void __exit blah_exit(void)
{
	printk(KERN_INFO "Bye bye!\n");
}

module_init(blah_init);
module_exit(blah_exit);

MODULE_AUTHOR("My name here");
MODULE_DESCRIPTION("blah!");
MODULE_LICENSE("GPL");
---

And this is what I got!

  CC [M]  /home/rostedt/c/modules/warning.o
/home/rostedt/c/modules/warning.c: In function 'warn_here':
/home/rostedt/c/modules/warning.c:14: warning: 'x' is used uninitialized in this function
  Building modules, stage 2.


Why the fsck isn't the func but_not_here not getting a warning for the
first use of printk??  If I remove the if statement it gives me the
warning.  Hell, that if statement isn't even entered (g = 0).

If you remove the warn_here function altogether, this module gets no
warnings!!!

OK, this really bothers me :(

btw, if you are wondering. I did load the module, and here's the output
from dmesg:

x=-124784640
y=-124784640

And I even tried it with removing warn me, compiled with no warnings and
then got this:

y=134514958

Huh!

-- Steve

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Stephen Hemminger]

On Wed, 10 May 2006 13:45:58 -0400 (EDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> Oh fsck! gcc is hosed. I just tried out this BS module:

Read the GCC bug report.
	http://gcc.gnu.org/bugzilla/show_bug.cgi?id=5035

It seems it is one of those "it too hard to fix, so we aren't going to"
blow offs.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Serge Belyshev]

Stephen Hemminger <shemminger@osdl.org> writes:

> On Wed, 10 May 2006 13:45:58 -0400 (EDT)
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> 
>> Oh fsck! gcc is hosed. I just tried out this BS module:
>
> Read the GCC bug report.
> 	http://gcc.gnu.org/bugzilla/show_bug.cgi?id=5035
>
> It seems it is one of those "it too hard to fix, so we aren't going to"
> blow offs.

That bug report has nothing to do with the issue above. The correct
PR is: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19430

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Adrian Bunk]

On Wed, May 10, 2006 at 01:45:58PM -0400, Steven Rostedt wrote:
> 
> Oh fsck! gcc is hosed. I just tried out this BS module:
>...
> And this is what I got!
> 
>   CC [M]  /home/rostedt/c/modules/warning.o
> /home/rostedt/c/modules/warning.c: In function 'warn_here':
> /home/rostedt/c/modules/warning.c:14: warning: 'x' is used uninitialized in this function
>   Building modules, stage 2.
> 
> 
> Why the fsck isn't the func but_not_here not getting a warning for the
> first use of printk??  If I remove the if statement it gives me the
> warning.  Hell, that if statement isn't even entered (g = 0).
>...

I can't reproduce this, gcc 4.1 gives me:

  CC [M]  init/test.o
init/test.c: In function 'warn_here':
init/test.c:14: warning: 'x' is used uninitialized in this function
init/test.c: In function 'but_not_here':
init/test.c:23: warning: 'y' is used uninitialized in this function

> -- Steve

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: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

On Wed, 10 May 2006, Adrian Bunk wrote:

>
> I can't reproduce this, gcc 4.1 gives me:
>
>   CC [M]  init/test.o
> init/test.c: In function 'warn_here':
> init/test.c:14: warning: 'x' is used uninitialized in this function
> init/test.c: In function 'but_not_here':
> init/test.c:23: warning: 'y' is used uninitialized in this function
>

OK, then it's fixed. I just noticed I'm using gcc 4.0.1

-- Steve

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Adrian Bunk]

On Wed, May 10, 2006 at 10:24:01PM +0200, Adrian Bunk wrote:
> On Wed, May 10, 2006 at 01:45:58PM -0400, Steven Rostedt wrote:
> > 
> > Oh fsck! gcc is hosed. I just tried out this BS module:
> >...
> > And this is what I got!
> > 
> >   CC [M]  /home/rostedt/c/modules/warning.o
> > /home/rostedt/c/modules/warning.c: In function 'warn_here':
> > /home/rostedt/c/modules/warning.c:14: warning: 'x' is used uninitialized in this function
> >   Building modules, stage 2.
> > 
> > 
> > Why the fsck isn't the func but_not_here not getting a warning for the
> > first use of printk??  If I remove the if statement it gives me the
> > warning.  Hell, that if statement isn't even entered (g = 0).
> >...
> 
> I can't reproduce this, gcc 4.1 gives me:
> 
>   CC [M]  init/test.o
> init/test.c: In function 'warn_here':
> init/test.c:14: warning: 'x' is used uninitialized in this function
> init/test.c: In function 'but_not_here':
> init/test.c:23: warning: 'y' is used uninitialized in this function

Same with gcc 4.0.

I can reproduce your problem only with gcc 3.3 and gcc 3.4.

Can we please discuss issues in current gcc releases instead of gcc 
bashing ("Oh fsck! gcc is hosed.") based on issues no longer present in 
the latest two major releases of gcc?

TIA
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: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

On Wed, 10 May 2006, Adrian Bunk wrote:

>
> Same with gcc 4.0.

I had a typo. it's 4.0.4

gcc version 4.0.4 20060422 (prerelease) (Debian 4.0.3-2)

But it is a Debian "prerelease".

>
> I can reproduce your problem only with gcc 3.3 and gcc 3.4.
>
> Can we please discuss issues in current gcc releases instead of gcc
> bashing ("Oh fsck! gcc is hosed.") based on issues no longer present in
> the latest two major releases of gcc?
>

Apologies to gcc.  It's no excuse, but I get a bit ansie when I'm late a
work debugging lots of problems.  Both work related and LKML related.
I think I took my frustrations out in that email.

So I don't unneccessarily insult anyone else, I'm calling it a day.

-- Steve

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

Someone emailed me the bug report for gcc:

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=5035


And in this, it showed the trick to initialize self to turn off that
warning.

Would it be OK to define a macro like:

#ifdef CONFIG_SHOW_ALL_UNINIT_WARNINGS
#  define init_self(x) x
#else
#  define init_self(x) x = x
#endif

Such that we can at least look at the places of bogus uninitialized
warnings and do something like:

Index: ipc/sem.c
===================================================================
--- ipc/sem.c   (revision 796)
+++ ipc/sem.c   (working copy)
@@ -809,7 +809,7 @@
 {
        struct sem_array *sma;
        int err;
-       struct sem_setbuf setbuf;
+       struct sem_setbuf init_self(setbuf);
        struct kern_ipc_perm *ipcp;

        if(cmd == IPC_SET) {


Seems to work.  And if you want to make sure that a place doesn't need it
anymore, use -Winit-self and have a script to do a full make once with
CONFIG_SHOW_ALL_UNINIT_WARNINGS and once with -Winit-self, and make sure
that all the -Winit-self warnings are in the
CONFIG_SHOW_ALL_UNINIT_WARNINGS. Otherwise the init_self isn't needed
anymore.

-- Steve

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

There's no code increase when you init something to itself . I could
convert all the instance of the warning, that I've investigated, to a
system like this . I think it would be a benefit so we could clearly
identify any new warnings added over time, and quiet the ones we know
aren't real errors .

However, from all the responses I'd imagine a patch like this wouldn't
get accepted ..

Daniel

On Wed, 2006-05-10 at 15:20 -0400, Steven Rostedt wrote:
> Someone emailed me the bug report for gcc:
> 
>  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=5035
> 
> 
> And in this, it showed the trick to initialize self to turn off that
> warning.
> 
> Would it be OK to define a macro like:
> 
> #ifdef CONFIG_SHOW_ALL_UNINIT_WARNINGS
> #  define init_self(x) x
> #else
> #  define init_self(x) x = x
> #endif
> 
> Such that we can at least look at the places of bogus uninitialized
> warnings and do something like:
> 
> Index: ipc/sem.c
> ===================================================================
> --- ipc/sem.c   (revision 796)
> +++ ipc/sem.c   (working copy)
> @@ -809,7 +809,7 @@
>  {
>         struct sem_array *sma;
>         int err;
> -       struct sem_setbuf setbuf;
> +       struct sem_setbuf init_self(setbuf);
>         struct kern_ipc_perm *ipcp;
> 
>         if(cmd == IPC_SET) {
> 
> 
> Seems to work.  And if you want to make sure that a place doesn't need it
> anymore, use -Winit-self and have a script to do a full make once with
> CONFIG_SHOW_ALL_UNINIT_WARNINGS and once with -Winit-self, and make sure
> that all the -Winit-self warnings are in the
> CONFIG_SHOW_ALL_UNINIT_WARNINGS. Otherwise the init_self isn't needed
> anymore.
> 
> -- Steve
> 
> 

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

On Wed, 10 May 2006, Daniel Walker wrote:

>
> There's no code increase when you init something to itself . I could
> convert all the instance of the warning, that I've investigated, to a
> system like this . I think it would be a benefit so we could clearly
> identify any new warnings added over time, and quiet the ones we know
> aren't real errors .
>
> However, from all the responses I'd imagine a patch like this wouldn't
> get accepted ..
>

I really don't see why it couldn't be added.  What's the problem with it?

I mean, I see lots of advantages, and really no disadvantages.

It can be done incrementally such that each change can be scrutinized to
make sure it really isn't a bug.

Once it is determined, not to be a bug, we add the macro to hide the
warning.  That way we lower the noise of warnings and look for places that
do have bugs.

It is a marker to let those, as Alan Cox suggested, look at them once in a
while to see if they are not really bugs.

Think about it.  If you get rid of all places that gcc incorrectly shows
something uninitialized, you can find the places that are truely bugs.
Once we get rid of all warnings, we can then turn off the macro to look
again to make sure none of them are problems.

The only disadvantage is that there's some macro wrapping a variable.  But
hell, that even was one of the advantages.  It's a marker that shows us
that gcc thinks it's not initialized properly.

And I already mentioned, that it wouldn't be too hard to see if any of the
marked variables no longer needs to be marked.

-- Steve

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 16:44 -0400, Steven Rostedt wrote:
> On Wed, 10 May 2006, Daniel Walker wrote:
> 
> >
> > There's no code increase when you init something to itself . I could
> > convert all the instance of the warning, that I've investigated, to a
> > system like this . I think it would be a benefit so we could clearly
> > identify any new warnings added over time, and quiet the ones we know
> > aren't real errors .
> >
> > However, from all the responses I'd imagine a patch like this wouldn't
> > get accepted ..
> >
> 
> I really don't see why it couldn't be added.  What's the problem with it?
> 
> I mean, I see lots of advantages, and really no disadvantages.

We are in complete agreement .. The only disadvantage is maybe we cover
up and real error , but that seems pretty unlikely .. Maybe I'll get
motivated while your sleeping ..

Daniel

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 02:11:54PM -0700, Daniel Walker wrote:
> > I really don't see why it couldn't be added.  What's the problem with it?
> > 
> > I mean, I see lots of advantages, and really no disadvantages.

Your vision is quite selective, then.
 
> We are in complete agreement .. The only disadvantage is maybe we cover
> up and real error

... which is more than enough to veto it.  However, that is not all.
Consider the following scenario:

1) gcc gives false positive
2) tosser on a rampage "fixes" it
3) code is chaged a month later
4) a real bug is introduced - one that would be _really_ visible to gcc,
with "is used" in a warning
5) thanks to aforementioned tosser, that bug remains hidden.

And that's besides making code uglier for no good reason, etc.

Consider that preemptively NAKed.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 22:20 +0100, Al Viro wrote:
> On Wed, May 10, 2006 at 02:11:54PM -0700, Daniel Walker wrote:
> > > I really don't see why it couldn't be added.  What's the problem with it?
> > > 
> > > I mean, I see lots of advantages, and really no disadvantages.
> 
> Your vision is quite selective, then.
>  
> > We are in complete agreement .. The only disadvantage is maybe we cover
> > up and real error
> 
> ... which is more than enough to veto it.  However, that is not all.
> Consider the following scenario:
> 
> 1) gcc gives false positive
> 2) tosser on a rampage "fixes" it
> 3) code is chaged a month later
> 4) a real bug is introduced - one that would be _really_ visible to gcc,
> with "is used" in a warning
> 5) thanks to aforementioned tosser, that bug remains hidden.

I don't really see anything new here .. The same sort of stuff can
happen in any code considered for inclusion .. That's what the review
process is for .

Real errors can be covered up any number of way ..

Daniel

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 02:33:42PM -0700, Daniel Walker wrote:
> > Consider the following scenario:
> > 
> > 1) gcc gives false positive
> > 2) tosser on a rampage "fixes" it
> > 3) code is chaged a month later
> > 4) a real bug is introduced - one that would be _really_ visible to gcc,
> > with "is used" in a warning
> > 5) thanks to aforementioned tosser, that bug remains hidden.
> 
> I don't really see anything new here .. The same sort of stuff can
> happen in any code considered for inclusion .. That's what the review
> process is for .
> 
> Real errors can be covered up any number of way ..

One last time: your kind of patches actually increases the odds of new bug
staying unnoticed.

If you really fill the urge to pull a Bunk, do it somewhere else, please -
the real thing is already more than sufficiently annoying.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 22:39 +0100, Al Viro wrote:
> On Wed, May 10, 2006 at 02:33:42PM -0700, Daniel Walker wrote:
> > > Consider the following scenario:
> > > 
> > > 1) gcc gives false positive
> > > 2) tosser on a rampage "fixes" it
> > > 3) code is chaged a month later
> > > 4) a real bug is introduced - one that would be _really_ visible to gcc,
> > > with "is used" in a warning
> > > 5) thanks to aforementioned tosser, that bug remains hidden.
> > 
> > I don't really see anything new here .. The same sort of stuff can
> > happen in any code considered for inclusion .. That's what the review
> > process is for .
> > 
> > Real errors can be covered up any number of way ..
> 
> One last time: your kind of patches actually increases the odds of new bug
> staying unnoticed.

Your using kind of a broad brush .. What do you mean "your kind of
patches" ?

> If you really fill the urge to pull a Bunk, do it somewhere else, please -
> the real thing is already more than sufficiently annoying.

Huh ?

Daniel

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 02:45:54PM -0700, Daniel Walker wrote:
> > One last time: your kind of patches actually increases the odds of new bug
> > staying unnoticed.
> 
> Your using kind of a broad brush .. What do you mean "your kind of
> patches" ?

"Just to make gcc STFU" variety you seem to be advocating in these threads.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

On Wed, 10 May 2006, Al Viro wrote:

> On Wed, May 10, 2006 at 02:11:54PM -0700, Daniel Walker wrote:
> > > I really don't see why it couldn't be added.  What's the problem with it?
> > >
> > > I mean, I see lots of advantages, and really no disadvantages.
>
> Your vision is quite selective, then.

And maybe so is yours.

>
> > We are in complete agreement .. The only disadvantage is maybe we cover
> > up and real error

I disagree here that it covers up any bug. See below.

>
> ... which is more than enough to veto it.  However, that is not all.
> Consider the following scenario:
>
> 1) gcc gives false positive
> 2) tosser on a rampage "fixes" it
> 3) code is chaged a month later
> 4) a real bug is introduced - one that would be _really_ visible to gcc,
> with "is used" in a warning
> 5) thanks to aforementioned tosser, that bug remains hidden.

What's the difference in seeing a warning in a compile, and noticing that
there's a wrapper around a variable?  In fact, that wrapper is more
of a flag that something might be broken than a warning that everyone is
use to seeing.  In step 3, the code that is changed, should either remove
the wrapper on the variable, or recompile with the wrapper defaulted to
warn.  The bug is never hidden due to the fact that you can keep the
warnings on.

So if you like to look for real warnings, you can compile it with the
false positives turned off, and if you don't trust the hidden warnings,
use a compile where the false positives stay on.  So it's now a choice to
which you perfer.  Not everyone likes to see a bunch of false positives,
and have to fight to find the real bugs.

As stated before, gcc (and any other program) is not perfect, and can't be
right all the time.  So it takes the side of aggressive warnings. But with
the help of the programmer, it only needs to warn on actual bugs.

This solution does _not_ hide the bug in step 5.  It only surpresses it to
those who don't care.

>
> And that's besides making code uglier for no good reason, etc.

The ugliness of the code, only helps to point out that there might be a
problem.  So instead of constantly weeding through warnings in search of
real bugs, you can look through the code once in a while to see if
something was missed.

>
> Consider that preemptively NAKed.
>

Preemptive strikes are usually never a good thing.

-- Steve

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Alan Cox]

On Mer, 2006-05-10 at 08:06 -0700, Daniel Walker wrote:
> > Because while the warning is present people will check it now and again.
> 
> But it's pointless to review it, in this case and for this warning .

Then why did you review it ? 

It reminds people that it does need checking, and ensures now and then
people do check that there isn't actually a bug there.

Alan

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 16:39 +0100, Alan Cox wrote:
> On Mer, 2006-05-10 at 08:06 -0700, Daniel Walker wrote:
> > > Because while the warning is present people will check it now and again.
> > 
> > But it's pointless to review it, in this case and for this warning .
> 
> Then why did you review it ? 

So I wouldn't have to see that warning again ..

> It reminds people that it does need checking, and ensures now and then
> people do check that there isn't actually a bug there.

I don't see a reason why it needs checking .. People are just going to
spin their wheel reviewing code that's been reviewed .. Maybe someone
like me will write a patch to fix this warning , and we'll see this
process happening all over again ..

Daniel

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 08:38:41AM -0700, Daniel Walker wrote:
> On Wed, 2006-05-10 at 16:39 +0100, Alan Cox wrote:
> > On Mer, 2006-05-10 at 08:06 -0700, Daniel Walker wrote:
> > > > Because while the warning is present people will check it now and again.
> > > 
> > > But it's pointless to review it, in this case and for this warning .
> > 
> > Then why did you review it ? 
> 
> So I wouldn't have to see that warning again ..
> 
> > It reminds people that it does need checking, and ensures now and then
> > people do check that there isn't actually a bug there.
> 
> I don't see a reason why it needs checking .. People are just going to
> spin their wheel reviewing code that's been reviewed .. Maybe someone
> like me will write a patch to fix this warning , and we'll see this
> process happening all over again ..

Don't.  Fix.  Correct.  Code.

Ever.  Because sooner or later you will paper over real bug.  It's far better
to reject patches that just make $TOOL to STFU than risk blind "fix" hiding
a real bug.

Unless you show a real codepath that leads to use without initialization
(and do that in commit message, so it could be verified as real issue),
these patches are worthless in the best case and dangerous in the worst
one.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 17:21 +0100, Al Viro wrote:

> Don't.  Fix.  Correct.  Code.
> 
> Ever.  Because sooner or later you will paper over real bug.  It's far better
> to reject patches that just make $TOOL to STFU than risk blind "fix" hiding
> a real bug.

Couldn't agree with you more .. But I don't want to see the warning
either ..

> Unless you show a real codepath that leads to use without initialization
> (and do that in commit message, so it could be verified as real issue),
> these patches are worthless in the best case and dangerous in the worst
> one.

Several of my patches have nothing to do with initialization .. 

Daniel

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 09:37:18AM -0700, Daniel Walker wrote:
> On Wed, 2006-05-10 at 17:21 +0100, Al Viro wrote:
> 
> > Don't.  Fix.  Correct.  Code.
> > 
> > Ever.  Because sooner or later you will paper over real bug.  It's far better
> > to reject patches that just make $TOOL to STFU than risk blind "fix" hiding
> > a real bug.
> 
> Couldn't agree with you more .. But I don't want to see the warning
> either ..

*shrug*
I hope that raw number of warnings is not used as job performance metrics.
All I can suggest is (a) watch for _changes_ in the warnings between revisions
and (b) get gcc folks fix the warning generation...

> > Unless you show a real codepath that leads to use without initialization
> > (and do that in commit message, so it could be verified as real issue),
> > these patches are worthless in the best case and dangerous in the worst
> > one.
> 
> Several of my patches have nothing to do with initialization .. 

s/codepath.*/bug being fixed/

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Daniel Walker]

On Wed, 2006-05-10 at 17:42 +0100, Al Viro wrote:

> s/codepath.*/bug being fixed/

There's several that aren't fixing bugs , but I still think they are
useful . 

For instance, I found several drivers that defined tables used when the
driver is defined as a module, but I was compiling the driver built-in
so the table showed as "unused" . 

I added

#ifdef MODULE

...

#endif /* MODULE */

How about those ? 

Daniel

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Alistair John Strachan]

On Wednesday 10 May 2006 17:37, Daniel Walker wrote:
> On Wed, 2006-05-10 at 17:21 +0100, Al Viro wrote:
> > Don't.  Fix.  Correct.  Code.
> >
> > Ever.  Because sooner or later you will paper over real bug.  It's far
> > better to reject patches that just make $TOOL to STFU than risk blind
> > "fix" hiding a real bug.
>
> Couldn't agree with you more .. But I don't want to see the warning
> either ..

Maybe adding "#warning GCC false-positive in this file, line blah." would be a 
suitable compromise. "Seeing" the bug isn't that big a deal, it's opening the 
file up to investigate it only to find it's been investigated a million times 
before and the same conclusions drawn..

-- 
Cheers,
Alistair.

Third year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Andrew Morton]

Al Viro <viro@ftp.linux.org.uk> wrote:
>
> Don't.  Fix.  Correct.  Code.
> 
>  Ever.  Because sooner or later you will paper over real bug.

I occasionally receive patches which generate new warnings, and those
warnings flag real bugs.  The developer simply missing the warning amongst
all the other crap.

It seems to especialy affect ia64 developers, whose build is especially
noisy.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 03:03:21PM -0700, Andrew Morton wrote:
> Al Viro <viro@ftp.linux.org.uk> wrote:
> >
> > Don't.  Fix.  Correct.  Code.
> > 
> >  Ever.  Because sooner or later you will paper over real bug.
> 
> I occasionally receive patches which generate new warnings, and those
> warnings flag real bugs.  The developer simply missing the warning amongst
> all the other crap.
> 
> It seems to especialy affect ia64 developers, whose build is especially
> noisy.

I know.  But that's the argument in favour of using diff, not shutting the
bogus warnings up...

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[David S. Miller]

From: Al Viro <viro@ftp.linux.org.uk>
Date: Wed, 10 May 2006 23:10:24 +0100

> But that's the argument in favour of using diff, not shutting the
> bogus warnings up...

IMHO, the tree should build with -Werror without exception.
Once you have that basis, new ones will not show up easily
and the hard part of the battle has been won.

Yes, people will post a lot of bogus versions of warning fixes, but
we're already good at flaming those off already :-)

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 03:31:29PM -0700, David S. Miller wrote:
> From: Al Viro <viro@ftp.linux.org.uk>
> Date: Wed, 10 May 2006 23:10:24 +0100
> 
> > But that's the argument in favour of using diff, not shutting the
> > bogus warnings up...
> 
> IMHO, the tree should build with -Werror without exception.
> Once you have that basis, new ones will not show up easily
> and the hard part of the battle has been won.
> 
> Yes, people will post a lot of bogus versions of warning fixes, but
> we're already good at flaming those off already :-)

Alternatively, gcc could get saner.  Seriously, some very common patterns
trigger that shit - e.g. function that returns error or 0 and stores
value to *pointer_argument in case of success.  It's a clear regression
in 4.x and IMO should be treated as gcc bug.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Andrew Morton]

Al Viro <viro@ftp.linux.org.uk> wrote:
>
> On Wed, May 10, 2006 at 03:31:29PM -0700, David S. Miller wrote:
> > From: Al Viro <viro@ftp.linux.org.uk>
> > Date: Wed, 10 May 2006 23:10:24 +0100
> > 
> > > But that's the argument in favour of using diff, not shutting the
> > > bogus warnings up...
> > 
> > IMHO, the tree should build with -Werror without exception.
> > Once you have that basis, new ones will not show up easily
> > and the hard part of the battle has been won.
> > 
> > Yes, people will post a lot of bogus versions of warning fixes, but
> > we're already good at flaming those off already :-)
> 
> Alternatively, gcc could get saner.  Seriously, some very common patterns
> trigger that shit - e.g. function that returns error or 0 and stores
> value to *pointer_argument in case of success.  It's a clear regression
> in 4.x and IMO should be treated as gcc bug.

Sure - it's sad and we need some workaround.

The init_self() thingy seemed reasonable to me - it shuts up the warning
and has no runtime cost.  What we could perhaps do is to make

#define init_self(x) (x = x)

only if the problematic gcc versions are detected.  Later, if/when gcc gets
fixed up, we use

#define init_self(x)	x

Or something.  Probably a more specific name than "init_self" is needed in
this case - something that indicates that it's a specific workaround for
specific gcc versions.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 04:05:48PM -0700, Andrew Morton wrote:
> Sure - it's sad and we need some workaround.
> 
> The init_self() thingy seemed reasonable to me - it shuts up the warning
> and has no runtime cost.  What we could perhaps do is to make
> 
> #define init_self(x) (x = x)
> 
> only if the problematic gcc versions are detected.  Later, if/when gcc gets
> fixed up, we use

Sorry, no - it shuts up too much.  Look, there are two kinds of warnings
here.  "May be used" and "is used".  This stuff shuts both.  And unlike
"may be used", "is used" has fairly high S/N ratio.

Moreover, once you do that, you lose all future "is used" warnings on
that variable.  So your ability to catch future bugs is decreased, not
increased.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Andrew Morton]

Al Viro <viro@ftp.linux.org.uk> wrote:
>
> On Wed, May 10, 2006 at 04:05:48PM -0700, Andrew Morton wrote:
> > Sure - it's sad and we need some workaround.
> > 
> > The init_self() thingy seemed reasonable to me - it shuts up the warning
> > and has no runtime cost.  What we could perhaps do is to make
> > 
> > #define init_self(x) (x = x)
> > 
> > only if the problematic gcc versions are detected.  Later, if/when gcc gets
> > fixed up, we use
> 
> Sorry, no - it shuts up too much.  Look, there are two kinds of warnings
> here.  "May be used" and "is used".  This stuff shuts both.  And unlike
> "may be used", "is used" has fairly high S/N ratio.
> 
> Moreover, once you do that, you lose all future "is used" warnings on
> that variable.  So your ability to catch future bugs is decreased, not
> increased.

Only for certain gcc versions.  Other people use other versions, so it'll
be noticed.  If/when gcc gets fixed, more and more people will see the real
warnings.

Look, of course it has problems.  But the current build has problems too. 
It's a question of which problem is worse..

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Wed, May 10, 2006 at 04:45:54PM -0700, Andrew Morton wrote:
> Al Viro <viro@ftp.linux.org.uk> wrote:
> > Sorry, no - it shuts up too much.  Look, there are two kinds of warnings
> > here.  "May be used" and "is used".  This stuff shuts both.  And unlike
> > "may be used", "is used" has fairly high S/N ratio.
> > 
> > Moreover, once you do that, you lose all future "is used" warnings on
> > that variable.  So your ability to catch future bugs is decreased, not
> > increased.
> 
> Only for certain gcc versions.  Other people use other versions, so it'll
> be noticed.  If/when gcc gets fixed, more and more people will see the real
> warnings.
> 
> Look, of course it has problems.  But the current build has problems too. 
> It's a question of which problem is worse..

FWIW, I've got mostly finished pile of scripts (still needs to be
consolidated, with merge into git for some parts) that does that following:
take two trees and build log for the first one; generate remapped log
with all lines of form <filename>:<line number>:<text> modified.  If line
in question survives in the new tree, turn it into
N:<new filename>:<new line number>:<text>
with new filename and line giving its new location, otherwise turn it into
O:<filename>:<line number>:<text>

That reduces the size of diff between build logs a _lot_ - basically,
all noise from changed line numbers is gone and we are left with real
changes.  It works better with git (we catch renames that way), but
even starting with diff between the trees works fairly well.

IME, it makes watching for regressions quite simple, even when dealing with
something like 2.6.16-rc2 and current, with shitloads of changes in between.
And yes, I _do_ watch ia64 tree, with all its noise.  Moreover, I watch
CHECK_ENDIAN sparse builds, aka thousands of warnings all over the tree...

I'll get that stuff into sane form and post it; IMO it solves the noise
problem just fine.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Steven Rostedt]

On Thu, 11 May 2006, Al Viro wrote:

>
> FWIW, I've got mostly finished pile of scripts (still needs to be
> consolidated, with merge into git for some parts) that does that following:
> take two trees and build log for the first one; generate remapped log
> with all lines of form <filename>:<line number>:<text> modified.  If line
> in question survives in the new tree, turn it into
> N:<new filename>:<new line number>:<text>
> with new filename and line giving its new location, otherwise turn it into
> O:<filename>:<line number>:<text>

And FWIW, not everyone has your scripts or the time to use them. The
init_self(x) thinging (and yes I hate that name) can be made to be a
compile time option.  So for those that are writing code, and only want to
look at the warnings that effect them, they have an option to turn off the
warnings of others (marked as false positives only). Heck, make the false
positives on as the default, and let others turn it off when developing.

I am one that spent several hours debugging my code, looking for a bug
that was caused by an "uninitialized variable".  The problem was, that I
became so damn use to ignoring those warnings, that I ignored them in my
own code.  I'm now a little more careful, but it still takes more time to
notice if those warnings are in code that I changed, or not.


>
> That reduces the size of diff between build logs a _lot_ - basically,
> all noise from changed line numbers is gone and we are left with real
> changes.  It works better with git (we catch renames that way), but
> even starting with diff between the trees works fairly well.
>
> IME, it makes watching for regressions quite simple, even when dealing with
> something like 2.6.16-rc2 and current, with shitloads of changes in between.
> And yes, I _do_ watch ia64 tree, with all its noise.  Moreover, I watch
> CHECK_ENDIAN sparse builds, aka thousands of warnings all over the tree...
>
> I'll get that stuff into sane form and post it; IMO it solves the noise
> problem just fine.

And IMO it solves it for you.  Not for everyone else.  Really, the only
argument that I see you have is the ugliness of the macro.  Since it can
be made configureable whether or not it disables those warnings, it
shouldn't affect you in any other way.  But it will really help those that
want to skip those warnings while looking at the compile output of their
code while they are developing it.

-- Steve

[PATCH -mm] introduce a false positive macro

[Steven Rostedt]

This is a patch to introduce a macro that can be used to suppress false
positives caused by GCC and looked at by a developer to turn off the
warning.

It does NOT turn off the warnings by default!!!

This is to give developers that usually ignore all the "uninitialized
variable" warings an opportunity to turn off these warnings to concentrate
on the ones that they make.

The only true arguement against this is that it makes the code not so
pretty.  Otherwise, it causes no more bloat and may even allow those to
debug better.

Now kernel hacking has an option for developers to turn off the warnings
that were looked at and marked.

This patch only introduces the macro.  If it is accepted, then more
patches need to be made to mark the variables.

Even if a variable is marked incorrectly as not a bug, it's still not
bad, in fact, I would say is better.  Only those that need to look at that
that code should keep this off.  Remember, this only turns off the
warnings if you explicitly do so yourself.

-- Steve

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Index: linux-2.6.17-rc3-mm1/include/linux/types.h
===================================================================
--- linux-2.6.17-rc3-mm1.orig/include/linux/types.h	2006-05-11 05:35:33.000000000 -0400
+++ linux-2.6.17-rc3-mm1/include/linux/types.h	2006-05-11 05:49:06.000000000 -0400
@@ -192,4 +192,15 @@ struct ustat {
 	char			f_fpack[6];
 };

+#ifdef CONFIG_HIDE_FALSE_POSITIVES
+/*
+ *  No parentheses around x = x  because
+ *    int (i=i);
+ *  doesn't compile.
+ */
+# define uninit_var(x) x = x
+#else
+# define uninit_var(x) x
+#endif
+
 #endif /* _LINUX_TYPES_H */
Index: linux-2.6.17-rc3-mm1/lib/Kconfig.debug
===================================================================
--- linux-2.6.17-rc3-mm1.orig/lib/Kconfig.debug	2006-05-11 05:37:58.000000000 -0400
+++ linux-2.6.17-rc3-mm1/lib/Kconfig.debug	2006-05-11 05:48:28.000000000 -0400
@@ -254,6 +254,22 @@ config FORCED_INLINING
 	  become the default in the future, until then this option is there to
 	  test gcc for this.

+config HIDE_FALSE_POSITIVES
+	bool "Hide gcc false positives of unititialized variables"
+	depends on DEBUG_KERNEL
+	help
+	  gcc sometimes shows that a variable is uninitialized when the logic
+	  actually does initialize it before use.  The kernel has lots of these
+	  warnings.  This option hides those warnings that were actually looked
+	  at by a human, and decided (right or wrong) that this variable is indeed
+	  properly initialized.
+
+	  If you are a developer that doesn't care about these warnings, and trust
+	  that the one that marked these variables, did so correctly.  Then you
+	  may turn on this option, to look for your own mistakes.
+
+	  Otherwise, say N
+
 config DEBUG_SYNCHRO_TEST
 	tristate "Synchronisation primitive testing module"
 	depends on DEBUG_KERNEL

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Adrian Bunk]

On Wed, May 10, 2006 at 04:45:54PM -0700, Andrew Morton wrote:
> Al Viro <viro@ftp.linux.org.uk> wrote:
> >
> > On Wed, May 10, 2006 at 04:05:48PM -0700, Andrew Morton wrote:
> > > Sure - it's sad and we need some workaround.
> > > 
> > > The init_self() thingy seemed reasonable to me - it shuts up the warning
> > > and has no runtime cost.  What we could perhaps do is to make
> > > 
> > > #define init_self(x) (x = x)
> > > 
> > > only if the problematic gcc versions are detected.  Later, if/when gcc gets
> > > fixed up, we use
> > 
> > Sorry, no - it shuts up too much.  Look, there are two kinds of warnings
> > here.  "May be used" and "is used".  This stuff shuts both.  And unlike
> > "may be used", "is used" has fairly high S/N ratio.
> > 
> > Moreover, once you do that, you lose all future "is used" warnings on
> > that variable.  So your ability to catch future bugs is decreased, not
> > increased.
> 
> Only for certain gcc versions.  Other people use other versions, so it'll
> be noticed.  If/when gcc gets fixed, more and more people will see the real
> warnings.
> 
> Look, of course it has problems.  But the current build has problems too. 
> It's a question of which problem is worse..

We could turn of this kind of warnings that generate these kind of false 
positives globally with -Wno-uninitialized until a future gcc version 
might be better at avoiding false positives.

But there's one problem, this turns off two kinds of warnings:
- 'foo' may be used uninitialized in this function
- 'foo' is used uninitialized in this function

The first kind of warnings is the one generating the false positives 
while the second kind are warnings we do not want to lose, but AFAIK 
there's no way to only turn off the first kind.

Perhaps asking the gcc developers for separate options for these two 
kinds of warnings in gcc 4.2 and then turning off the first kind is 
the way to go?

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: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Al Viro]

On Thu, May 11, 2006 at 10:40:33PM +0200, Adrian Bunk wrote:
> We could turn of this kind of warnings that generate these kind of false 
> positives globally with -Wno-uninitialized until a future gcc version 
> might be better at avoiding false positives.
> 
> But there's one problem, this turns off two kinds of warnings:
> - 'foo' may be used uninitialized in this function
> - 'foo' is used uninitialized in this function
> 
> The first kind of warnings is the one generating the false positives 
> while the second kind are warnings we do not want to lose, but AFAIK 
> there's no way to only turn off the first kind.
> 
> Perhaps asking the gcc developers for separate options for these two 
> kinds of warnings in gcc 4.2 and then turning off the first kind is 
> the way to go?

Folks, let's look at it that way:
	* warnings are tools to locate broken places in the tree.
	* we have two signals ("is unused" and "may be unused"), say
A(location, verision) and B(location, version).
	* A has fairly high S/N ratio.
	* B has very large noise component, but it's only weakly dependent
on the verision.

The question is how to get useful information out of those.
	* solution 1: introduce C(location, version) and filter A and B
with it, to kill noise in B.
	* solution 2: ignore B, either by gcc modification or simply filtering
it with grep.
	* solution 3: subtract the signals for consequent versions.

IMO it's obvious that combining outputs of (2) and (3) gives the best S/N.
The reason why (1) is bad is that it kills high-S/N channel in the areas
with high noise on low-S/N channel *and* that filtered-out areas will
remain filtered out in the next versions.  IOW, it's a clear loss.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Roland Dreier]

    David> IMHO, the tree should build with -Werror without exception.
    David> Once you have that basis, new ones will not show up easily
    David> and the hard part of the battle has been won.

It's a great goal, but which gcc version and architecture do you
declare has to build the kernel with -Werror?  Every gcc version and
platform produces a different set of warnings.  And what do you do
when all released versions of gcc produce a false positive warning?

The problem is that fixing false positive warnings often leads people
to write unnatural code that may hide future bugs (and in fact may be
buggy even when written).

 - R.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[David S. Miller]

From: Andrew Morton <akpm@osdl.org>
Date: Wed, 10 May 2006 15:03:21 -0700

> I occasionally receive patches which generate new warnings, and those
> warnings flag real bugs.  The developer simply missing the warning amongst
> all the other crap.

I totally agree.  These gcc-4.x warnings bug this shit out of
me too and we should fix them now.

Even a huge tree like gcc can build itself %100 fine with -Werror, for
pretty much every configuration possible, and yet we're astronomically
so far away from that.  That's totally unacceptable.

Re: [PATCH -mm] sys_semctl gcc 4.1 warning fix

[Mike Galbraith]

On Wed, 2006-05-10 at 15:30 -0700, David S. Miller wrote:
> Even a huge tree like gcc can build itself %100 fine with -Werror, for

What's the emoticon for coffee going down your windpipe?

	-Mike