On Fri, 15 Apr 2005, Matthew Wilcox wrote:
> On Fri, Apr 15, 2005 at 10:03:05PM +1000, Herbert Xu wrote:
> > I suppose it could be smart and stay quiet about
> >
> > val < 0 || val > BOUND
> >
> > However, gcc is slow enough as it is without adding unnecessary
> > smarts like this.
>
> It only warns with -W on, not with -Wall, so I see no compelling
> reason to fix this.
Fixing the -W warning was not the main point. The main point was simply
that the check makes no sense at all.
> I think the real problem here is that 'arg'
> is declared 2 pages earlier in the function prototype (aka the
> function-growth-hormone-imbalance syndrome).
>
> There's two good ways of fixing this, adding a f_setsig() function:
>
[...]
> or add a function that checks a variable to see if it's a valid signal number:
[...]
>
> Looks like futex.c, ptrace.c, signal.c, sys.c and almost every
> architecture's ptrace code could easily make use of the latter, but not
> the former. It also looks like we have a few off-by-one errors. For
[...]
> so I'd recommend the second solution.
Thank you for your feedback, that makes a lot of sense. That should get
rid of the pointless tests, get rid of the -W warning and get rid of the
off-by-one errors - sounds good to me.
I'll create patches and send them along shortly.
> But be careful not to "fix up"
> cases like:
>
> ./kernel/exit.c: if (sig < 1 || sig > _NSIG)
>
> where we really don't want to allow zero.
>
I'll watch out for those.
I can either leave them alone or re-write as one of
if (valid_signal(sig) && sig != 0)
if (valid_signal(sig) && sig > 0)
any preference?
Personally I would probably go with the
'rewrite as if (valid_signal(sig) && sig > 0)' one to encourage use of
the new valid_signal() function and make usage consistent.
--
Jesper Juhl
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
