[PATCH 05/18] Fix 64bit arch issue in synaptics eventcomm.c

Mark Kettenis mark.kettenis at xs4all.nl
Fri Oct 8 13:51:08 PDT 2010


> Date: Fri, 08 Oct 2010 21:36:02 +0200
> From: Takashi Iwai <tiwai at suse.de>
> 
> At Fri, 8 Oct 2010 20:48:10 +0200 (CEST),
> Mark Kettenis wrote:
> > 
> > > From: Takashi Iwai <tiwai at suse.de>
> > > Date: Fri,  8 Oct 2010 19:22:29 +0200
> > 
> > Sorry, but it isn't obvious to me what issue this fixes.
> 
> In C, "1" is an integer, not an unsigned long.
> Thus (1 << 33) doesn't give you the 33th bit shift, but it's undefined.

But if you'd actually use 33 (or event 32) as an offset, things
wouldn't work on a 32-bit platform would they?

Anyway,

> If any, this must be (1UL << 32). 

This is the idiom that is much more common.  I probably wouldn't have
questioned things if you'd written it like that.  I recommend you to
stick to this version.

> Also, it'd be better if such a test macro returns 1 instead of a
> random non-zero value.  So does my patch.

In C this doesn't matter.

> (Yeah, I know the changelogs are missing in most of patches;
>  as mentioned, it's just flushing of initial patches ;)
>  I'll rewrite and resubmit after reviews.)

Well, it doesn't help your reviewers does it?


More information about the xorg-devel mailing list