Re: [PATCH] sim: tweak signed to unsigned local vars

[Mike Frysinger <vapier at gentoo dot org>]

On Monday 12 April 2010 12:06:18 Joel Brobecker wrote:
> > > I'm not sure I agree on this one. "tmp" is used to store the result of
> > > a subtraction of 2 pointers. IIRC, the exact type returned is
> > > ptrdiff_t, which is a signed value...
> > 
> > true, but the math should never yield a negative value.  the compare is
> > between a base pointer and a pointer returned from strchr() on the base
> > pointer.  so the value should always be >= 0 and it should always fit in
> > 32bits.
> 
> I always tend to be very careful with this type of reasoning, probably
> because of my past writing safety-critical software (and also how
> surprisingly difficult it turned out to be to formally prove that
> a piece of code would never overflow).
> 
> Even if what you are saying is true, I think that we'll have less
> conversion issues if we use the proper types.  But that's just me.
> Perhaps others with more C experience than I do will agree with you
> that we're fussing over something that actually does not matter.

this is a string passed via command line option.  if it were even remotely 
close to the size required to overflow ("unsigned" -> "unsigned int" -> 32bit 
on all systems where this socket code is going to be used), something is 
terribly wrong elsewhere.  even if we grant it 16bit storage, a 65k string 
specifying a host:port makes no sense, especially when many systems have a 
hard time swallowing a command line over 32k.

i dont have a problem coding defensively when it makes sense, but i cant see 
any such requirements in any of the sim code.

further, while ptrdiff_t may make sense since we're diffing two pointers, the 
value is passed straight into strncpy() which interprets the value as size_t.  
so if we were going to spend the time on this (which i dont think this code 
warrants), a lot more code needs to change than the variable type.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.