lkml.org 
[lkml]   [2004]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Remove pointless <0 comparison for unsigned variable in fs/fcntl.c
Matthew Wilcox wrote:
> On Sun, Nov 21, 2004 at 11:55:23PM +0100, Jesper Juhl wrote:
>
>>This patch removes a pointless comparison. "arg" is an unsigned long, thus
>>it can never be <0, so testing that is pointless.
>>
>>Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
>>
>> case F_SETSIG:
>> /* arg == 0 restores default behaviour. */
>>- if (arg < 0 || arg > _NSIG) {
>>+ if (arg > _NSIG) {
>> break;
>
>
> I've seen patches like this before. I'm generally in favour of removing
> the unnecessary test, but Linus rejected it on the grounds the compiler
> shouldn't be warning about it and it's better to be more explicit about
> the range test. Maybe he's changed his mind between then and now ;-)
>
Let's find out.
Linus, would you accept patches like this?

I've been building recent kernels with -W and there are tons of places
with similar comparisons like the one above, as well as places where
signed and unsigned values are compared, places where values are
potentially truncated in signed/unsigned assignments and similar.
At the very least a review of these code locations to make sure they are
all safe would make sense, and I think it would also make sense to get
rid of the comparisons that always evaluate true or false due to the
signedness or range of datatypes.
I probably won't be able to properly evaluate/review *all* the instances
of this in the kernel, but I don't mind spending some time reviewing
what I can and submit patches, but I don't want to waste my time with it
if you already know up-front that you'll just be dropping all such
patches. Or is this up to the individual maintainers to accept/reject?
If so, then I'll go ahead and submit patches, then the individual
maintainers can ack/nack as they please.

Comments?


--
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-03-22 14:08    [W:0.083 / U:0.724 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site