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

[Mike Frysinger <vapier at gentoo dot org>]

On Monday 12 April 2010 11:24:13 Joel Brobecker wrote:
> > diff --git a/sim/common/dv-sockser.c b/sim/common/dv-sockser.c
> > index 643ce34..18987f9 100644
> > --- a/sim/common/dv-sockser.c
> > +++ b/sim/common/dv-sockser.c
> > @@ -129,7 +129,8 @@ dv_sockser_init (SIM_DESC sd)
> > 
> >    struct sockaddr_in sockaddr;
> >    char hostname[100];
> >    const char *port_str;
> > 
> > -  int tmp,port;
> > +  unsigned tmp;
> > +  int port;
> 
> 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.

> >  hw_port_encode (struct hw *me,
> >  
> >  		int port_number,
> >  		char *buf,
> > 
> > -		int sizeof_buf,
> > +		unsigned sizeof_buf,
> > 
> >  		port_direction direction)
> 
> Would type size_t work, in this case?

size_t would be usable in pretty much all the places i changed, but i 
consciously did not pick that because "unsigned" is the current convention, 
both with local vars and function arguments.  i didnt want to desync the type 
conventions where some used size_t and some used unsigned especially since 
they're different sizes on 64bit systems.

i certainly wouldnt be opposed to someone mass converting the source to 
size_t, but i dont think i'm going to volunteer for that ;)

> > @@ -583,7 +583,7 @@ hw_add_range_array_property (struct hw *me,
> > 
> >  			   * sizeof (unsigned_cell));
> >    
> >    unsigned_cell *cells = hw_zalloc (me, sizeof_cells);
> >    unsigned_cell *cell;
> > 
> > -  int i;
> > +  unsigned i;
> 
> I am not sure about this one either. Can you try changing the type
> of parameter nr_ranges to int? It looks like the only usage of this
> function is in hw-tree, and the variable used is actually an int...

i'll check these
-mike

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