Re: [patch] x86, UV: integer wrap bug in uv_hub_ipi_value()

[Russ Anderson]

On Tue, Nov 20, 2012 at 11:10:55AM +0000, Alan Cox wrote:
> On Tue, 20 Nov 2012 07:28:56 +0300 Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> > On Mon, Nov 19, 2012 at 06:48:34PM -0600, Russ Anderson wrote:
> > > On Sat, Nov 17, 2012 at 06:16:11PM +0300, Dan Carpenter wrote:
> > > > This is a static checker fix.  The problem is that we store the bits
> > > > from "uv_apicid_hibits" into "apicid" (the high 16 bits) but then we
> > > > shift it 16 bit to the left.  "apicid" is an int so it wraps and we lose
> > > > them.
> > > 
> > > Is this the complete patch?  phys_apicid is an int, but gets
> > > cast as unsigned long.  Doesn't phys_apicid also have to be
> > > changed to unsigned long?  And why ulong instead of uint (on x86_64)?
> > 
> > Uint is 32bit across all arches in linux and unix, according to
> > wikipedia. 
> 
> For Linux yes, if Wackypedia claims it for "unix" it's wrong 8) and varies
> between 16 and 36bits that I can think of.

64 bit int on old Cray cpus (unicos).  :-)

> If you specifically want 32bits use "u32", it's not going to make any
> actual difference to unsigned int but it makes the requirement explicit.

I very much agree.  I prefer u32, u64 (etc) because they are
unambiguous.  It removes all doubt as to the actual meaning.

Conversly, the fact that "long" has different meanings makes
it at best problematic.  Was the code written assuming "long"
was 32 or 64 bits?  Having data types that can have different
sizes is just asking for trouble.

-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@xxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/