signed vs unsigned issues in the code

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

signed vs unsigned issues in the code

Jörg Sommer
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?

Regards, Jörg.
--
Gott hat den Menschen erschaffen, weil er vom Affen enttäuscht war.
Danach hat er auf weitere Experimente verzichtet.
                                                          (Mark Twain)
_______________________________________________
gnucash-user mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-user
-----
Please remember to CC this list on all your replies.
You can do this by using Reply-To-List or Reply-All.
Reply | Threaded
Open this post in threaded view
|

Re: 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
_______________________________________________
gnucash-user mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-user
-----
Please remember to CC this list on all your replies.
You can do this by using Reply-To-List or Reply-All.