linux-kernel - Re: [PATCH -mm] timer: parenthesis fix in tbase_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a781481a0705091129t6a348fb7ya2a225123fca3f6@mail.gmail.com>
Date:	Wed, 9 May 2007 23:59:39 +0530
From:	"Satyam Sharma" <satyam.sharma@...il.com>
To:	"Pallipadi, Venkatesh" <venkatesh.pallipadi@...el.com>
Cc:	"Jarek Poplawski" <jarkao2@...pl>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, "Oleg Nesterov" <oleg@...sign.ru>
Subject: Re: [PATCH -mm] timer: parenthesis fix in tbase_get_deferrable() etc.

On 5/9/07, Pallipadi, Venkatesh <venkatesh.pallipadi@...el.com> wrote:
>
> >-----Original Message-----
> >From: Jarek Poplawski [mailto:jarkao2@...pl]
> >Sent: Tuesday, May 08, 2007 10:32 PM
> >To: Andrew Morton
> >Cc: Pallipadi, Venkatesh; linux-kernel@...r.kernel.org; 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 <jarkao2@...pl> wrote:
> >>
> >> >
> >> > Signed-off-by: Jarek Poplawski <jarkao2@...pl>
> >> >
> >> > ---
> >> >
> >> > 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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/