In mm/slab.c in the function kmem_cache_create, there's a check for
'offset < 0' that is completely unnessesary since 'offset' is of
type size_t, and size_t is an unsigned datatype in all archs.
The patch below removes this un-needed code - even gcc agrees that this
code is not needed and that case can never happen.
--- linux-2.6.1-rc1-mm2-orig/mm/slab.c 2004-01-06 01:33:09.000000000 +0100
+++ linux-2.6.1-rc1-mm2/mm/slab.c 2004-01-08 02:08:33.000000000 +0100
@@ -1042,7 +1042,7 @@ kmem_cache_create (const char *name, siz
(size < BYTES_PER_WORD) ||
(size > (1<<MAX_OBJ_ORDER)*PAGE_SIZE) ||
(dtor && !ctor) ||
- (offset < 0 || offset > size))
+ (offset > size))
BUG();
#if DEBUG
Patch is compile tested, that's all - but it seems to be 'obviously
correct' to me.
Kind regards,
Jesper Juhl
PS. CC'ing the people mentioned in the comments of mm/slab.c since I
couldn't fine any single person responsible for this file in MAINTAINERS -
hope that's OK.
On Wed, 2004-01-07 at 17:20, Jesper Juhl wrote:
> size_t is an unsigned datatype in all archs.
There are literally hundreds of these in the code.
Compile with -W -Wall and see for yourself.
On Wed, 7 Jan 2004, Joe Perches wrote:
> On Wed, 2004-01-07 at 17:20, Jesper Juhl wrote:
> > size_t is an unsigned datatype in all archs.
>
> There are literally hundreds of these in the code.
>
> Compile with -W -Wall and see for yourself.
>
Well, anything wrong in cleaning them up?
-- Jesper Juhl
Jason asked:
> Well, anything wrong in cleaning them [unsigned compare warnings] up?
It's more important that we write code that will fit in our limited
human brains than that we write code that will avoid spurious warnings
from gcc ('spurious' meaning warnings for code that gcc will correctly
compile anyway).
Or, see a couple months ago, in a thread with the Subject of:
[PATCH] irda: fix type of struct irda_ias_set.attribute.irda_attrib_string.len
in which Linus wrote:
> That's why I hate the "sign compare" warning of gcc so much - it warns
> about things that you CANNOT sanely write in any other way. That makes
> that particular warning _evil_, since it encourages people to write crap
> code.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373
On Thu, 2004-01-08 at 02:16, Paul Jackson wrote:
> Jason asked:
> > Well, anything wrong in cleaning them [unsigned compare warnings] up?
In this case the warning is not unsigned compare but
"comparison of .* is always [true|false]".
This sort of code generally makes me think someone did something wrong,
not just that the person added additional unnecessary checking.
On Thu, Jan 08 2004, Paul Jackson wrote:
> Jason asked:
> > Well, anything wrong in cleaning them [unsigned compare warnings] up?
>
> It's more important that we write code that will fit in our limited
> human brains than that we write code that will avoid spurious warnings
> from gcc ('spurious' meaning warnings for code that gcc will correctly
> compile anyway).
>
> Or, see a couple months ago, in a thread with the Subject of:
>
> [PATCH] irda: fix type of struct irda_ias_set.attribute.irda_attrib_string.len
>
> in which Linus wrote:
> > That's why I hate the "sign compare" warning of gcc so much - it warns
> > about things that you CANNOT sanely write in any other way. That makes
> > that particular warning _evil_, since it encourages people to write crap
> > code.
That's fine and has its place, no doubt about that. It doesn't cover the
patch in this thread though. The check is dead code. It's a cosmetic
problem though, gcc should not generate the code checking for < 0.
--
Jens Axboe
Jens Axboe wrote:
>That's fine and has its place, no doubt about that. It doesn't cover the
>patch in this thread though. The check is dead code. It's a cosmetic
>problem though, gcc should not generate the code checking for < 0.
>
>
I'll fix it when I touch that area again. Probably with the patch that
allows constructors to fail, which would be 2.7 stuff. It's not really
urgent.
--
Manfred
Joe Perches wrote:
> On Thu, 2004-01-08 at 02:16, Paul Jackson wrote:
>
>>Jason asked:
>>
>>>Well, anything wrong in cleaning them [unsigned compare warnings] up?
>
>
> In this case the warning is not unsigned compare but
> "comparison of .* is always [true|false]".
>
> This sort of code generally makes me think someone did something wrong,
> not just that the person added additional unnecessary checking.
Agreed, often muddy thinking.
--
bill davidsen <[email protected]>
CTO TMR Associates, Inc
Doing interesting things with small computers since 1979