Token.c appears to have a bug.

jw schultz jw at pegasys.ws
Tue Oct 14 10:55:52 EST 2003


On Mon, Oct 13, 2003 at 05:41:56PM -0700, Wayne Davison wrote:
> On Mon, Oct 13, 2003 at 04:54:20PM -0700, jw schultz wrote:
> > Having looked at that bit of code now.  I am a bit concerned about the
> > use of shift operators on signed integers here.
> 
> I don't see the problem with regard to this code because we're only
> looking at bits that where known to exist in the var before the shift
> (i.e. no sign-extended bits have any effect).  Also, your proposed
> change doesn't affect the shifting at all -- you'd have to cast the "n"
> to unsigned before shifting it, not cast the result to unsigned after
> shifting it.  Since the code we're calling is expecting a "char *", I
> think we should leave the type of temp_byte unchanged.

Sorry, the full extent of the signedness (in particular n)
didn't hit me until after i had already created the
half-a**ed patch.  I've little doubt that the sign-extension
won't come into play simply because we never touch the bits
that would have resulted.  "concerned" is perhaps an
overstatement but i'm not sure quite what term to use,
discomfited perhaps.  The fact that it works is primary.

> > Habit also makes me shy of relying on char being 8 bits but that is
> > probably overcautious.
> 
> I think a lot of things would fail in the code if "char" wasn't 8 bits.

Too true.  I'm not aware of any modern platforms where char
isn't 8 bits.  But i do recall one or two old architectures
where char was 12 or 9 bits and nothing says it cannot be 16
bits or even larger.


-- 
________________________________________________________________
	J.W. Schultz            Pegasystems Technologies
	email address:		jw at pegasys.ws

		Remember Cernan and Schmitt



More information about the rsync mailing list