On 5/9/07, Pallipadi, Venkatesh <[email protected]> wrote:
>-----Original Message-----
>From: Jarek Poplawski [mailto:[email protected]]
>Sent: Tuesday, May 08, 2007 10:32 PM
>To: Andrew Morton
>Cc: Pallipadi, Venkatesh; [email protected]; Oleg Nesterov
>Subject: Re: [PATCH -mm] timer: parenthesis fix in
>tbase_get_deferrable() etc.
>
>On Tue, May 08, 2007 at 04:33:58PM -0700, Andrew Morton wrote:
>> On Tue, 8 May 2007 12:33:48 +0200
>> Jarek Poplawski <[email protected]> wrote:
>>
>> >
>> > Signed-off-by: Jarek Poplawski <[email protected]>
>> >
>> > ---
>> >
>> > diff -Nurp 2.6.21-mm1-/kernel/timer.c 2.6.21-mm1/kernel/timer.c
>> > --- 2.6.21-mm1-/kernel/timer.c 2007-05-08
>11:54:48.000000000 +0200
>> > +++ 2.6.21-mm1/kernel/timer.c 2007-05-08
>12:05:11.000000000 +0200
>> > @@ -92,24 +92,24 @@ static DEFINE_PER_CPU(tvec_base_t *, tve
>> > /* Functions below help us manage 'deferrable' flag */
>> > static inline unsigned int tbase_get_deferrable(tvec_base_t *base)
>> > {
>> > - return ((unsigned int)(unsigned long)base &
>TBASE_DEFERRABLE_FLAG);
>> > + return (unsigned int)((unsigned long)base &
>TBASE_DEFERRABLE_FLAG);
>> > }
>...
>> The change makes sense, but does it actually "fix" anything?
>>
>
>Yes - this first place fixes logical error, so it's a sin
>- even if not punishable in practice. (It's also unnecessary
>test for long to int conversion.)
>
I am sorry, I don't understand. What is the logical error in the first
one?
Actually, your change makes it different from what was originally
indended.
Original intention was to type convert base to a 32 bit value and
bitwise& with FLAG.
But that is not what the original code is doing. If you wanted to
typecast "base" to "a 32 bit value" then you should've used u32
instead.
Anyway, if you originally intended to actually typecast "base" to
unsigned int, then you could do that directly without typecasting it
first to unsigned long (unnecessarily) and then to unsigned int. Of
course, if your system implements a pointer as something bigger than
unsigned int (which is what you eventually convert "base" to), then
you're screwed anyway and the intermediate typecast to unsigned long
doesn't buy you anything at all.
The other 3 changes in this patch were clearly meaningless, though.
-
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]
