This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


> > 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.

> 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 actually think, from the little that I have seen, that the code is
actually pretty confused on whether to use signed or unsigned. Again,
perhaps it's my Ada background where using the proper types is super
important (and an invaluable help), but, never mind - if you can make it
work with unsigned, then this is fine with me. You shouldn't have to pay
for the sins of others.

-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]