Re: [dev] [st] [PATCH] Explicit cast in CEIL macro

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.com>
Date: Tue, 24 Jun 2014 08:50:01 +0200

> No, this is wrong.
> Check cwscale and chscale in config.h, which are floats.
>
> xw.cw = CEIL(dc.font.width * cwscale);
> xw.ch = CEIL(dc.font.height * chscale);
>
> dc.font.width and dc.font.height are integers, but CEIL exlusively
> deals with floats.

Uhmmm, you are right (and it is a bit stupid the ceil of a integer
value ^^!).

> However, after further investigation, I also conclude including math.h
> is not necessary.

Well, it is not necessary to include math.h if you want to use ceil,
you can put only the prototype of ceil (it is explicitly allowed in
the standard), but I think we are not going to win anything using
ceil. So I agree with keeping CEIL macro.

The next point is to see is CEIL is correct:

        CEIL(x) (((x) != (int) (x)) ? (x) + 1 : (x))

It fails with negative numbers:

        CEIL(-3.3) => (-3.3 != -3) ? -3.3 + 1 : 3.3 => -2.3

it fails with positive numbers.

        CEIL(3.3) => (3.3 != 3) ? 3.3 + 1 : 3.3 => 4.3

I think a correct version may be:

        CEIL(x) ((int) (x) + ((x) > 0 ? 1.0 : 0.0))

Positive:
        CEIL(3.3) => 3 + (3.3 > 0 ? 1.0 : 0.0) => 4.0

Negative:
        CEIL(-3.3) => -3 + (-3.3 > 0 ? 1.0 : 0.0) => -3

Regards,

-- 
Roberto E. Vargas Caballero
Received on Tue Jun 24 2014 - 08:50:01 CEST

This archive was generated by hypermail 2.3.0 : Tue Jun 24 2014 - 09:00:07 CEST