signed vs unsigned issues in the code

[Chris Shoemaker]

On Mon, Aug 14, 2006 at 07:39:04PM +0000, Jörg Sommer wrote:
> Hi,
> 
> I've build GnuCash with -Wextra and got some warnungs about sign issues.
> 
> For example, look at gnc-date.c:234. The function name timespec_abs()
> suggests the value should be made a positive value, if it's negative. But
> the comparison "retval.tv_sec < 0" is always false, due to tv_sec is
> unsigned according gnc-date.h:
> ,----
> |  * struct timespec64 is just like the unix 'struct timespec' except 
> |  * that we use a 64-bit
> |  * unsigned int to store the seconds.  This should adequately cover
> |  ...
> |  typedef struct timespec64 Timespec;
> `----
> 
> So, somehere you assign an negative value to tv_sec of type (unsigned)
> int. The timespec_abs() function interprets this (signed) negativ value
> as (unsigned) positiv value---you know, (unsigned int)-1 == UINT_MAX.
> This function, except of the call of timespec_normalize()---which also
> makes mistakes---is a big noop.
> 
> Or look at fin.c:1212
> ,----
> | static double
> | rnd (double x, unsigned places)
> | {
> |  ...
> |   if (places >= 0)
> |   {
> |     sprintf (buf, "%.*f", (int) places, x);
> |  ...
> |   else
> `----
> 
> Isn't it stupid to check a non-negativ value, if it is greater or equal
> zero? This holds everytime.
> 
> This may not lead to an error and gcc removes such unreachable parts, but
> somewhere might be an assignment of an negative value to this
> variable/parameter, which than results in a really curious call of
> sprintf()?the case that should be prevented with the check.
> 
> I found this issues in gnucash 2.0.1. What do you think?

Thank you for the comments.  I think emails like this are welcome on
gnucash-devel, and patches are even better. :)

-chris