lkml.org 
[lkml]   [2005]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-04-17 23:01    [W:0.081 / U:0.688 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site