Unit tests fail on 32-bit.

[Traumflug]

From IRC:

[16:51] <bert> ERROR:pcb-printf.c:780:pcb_printf_test_unit: assertion failed (c[0] == 1000000000): (999999999 == 1000000000)

[16:51] <bert> /bin/sh: line 5: 3554 Aborted (core dumped) ${dir}$tst

[16:51] <bert> FAIL: unittest

It happens on 32-bit machines, only. Using the --enable-coord32 configure flag on a 64-bit machine doesn't expose the bug, either.

[Traumflug (mah-jump-ing)]

Created a branch with diagnostic code, LP1500224.

On a 64-bit machine it reports

- - - - -
/pcb-printf/test-unit:

sizeof(double) = 8
suffix m in_suffix m printf_code f
scale_factor 0.001000 default_prec 5
step_tiny 0.000500 step_small 0.005000
step_medium 0.025000 step_huge 2.500000
base 1000000.000000
result 1000000000.000000
Coord result 1000000000
OK
/pcb-printf/test-printf: OK
- - - - -

On a 32-bit machine it reports

- - - - -
sizeof(double) = 8
suffix m in_suffix m printf_code f
scale_factor 0.001000 default_prec 5
step_tiny 0.000500 step_small 0.005000
step_medium 0.025000 step_huge 2.500000
base 1000000.000000
result 1000000000.000000
Coord result 1000000000
**
ERROR:pcb-printf.c:810:pcb_printf_test_unit: assertion failed (c[0] == 1000000000): (999999999 == 1000000000)
/bin/sh: line 5: 12630 Aborted (core dumped) ${dir}$tst
FAIL: unittest
==================
1 of 1 test failed
==================
- - - - -

Weird. The diagnostic code should do the exactly same as the unit test.

[Sergey Stepanov (no-such-process)]

The problem is in the end of unit_to_coord() function. There is a floating point division at the end: `return base / unit->scale_factor;` which is then silently casts to integer and result is returned.
Casting to integer is done by truncating of fractional part, thus giving us this 999.99 -> 999

I did't dive deep into this (maybe this is highly compiler dependent), but i have simple proof-of-concept code, that gives the same results (with gcc for 32- and 64-bit targets):
999999999 in 32-bit mode
1000000000 in 64-bit mode
-------
file1.c
#include <stdio.h>
extern double unit_to_coord(double a);
int main()
{
    printf("%d\n",(int)get(1000000.0));
    return 0;
}
-------
file2.c
double unit_to_coord(double a)
{
    return a / 0.001; /* the same factor as for 'm' unit */
}
------

so, the quick fix is to use round() function in the unit_to_coord():

return round(base / unit->scale_factor);

[Traumflug (mah-jump-ing)]

Hmm. The diagnostic code does this cast, too. Just not silent, but explicitely:

  printf("Coord result %ld\n", (Coord)(base / u.scale_factor));

So I added such an explicit cast, see branch LP15000224. It'd nice f you could test this on 32-bit. If this still doesn't work I fear we have to use round().

[Sergey Stepanov (no-such-process)]

I run `make check` on branch LP15000224 in CentOS 6.7 32-bit. Test failed as in comment #1.

> I fear we have to use round().

I think there should be no fear to round floating point value before casting/converting it to integer (no matter if casting is implicit or explicit).
I call it best practice :).

There are some rounding functions returning rounded integer: "lrint, lrintf, lrintl, llrint, llrintf, llrintl - round to nearest integer", so one may use them.

[Traumflug (mah-jump-ing)]

A call to round() costs time, that's why I "fear" it. OK, maybe I'm a bit haunted by my work on AVR embedded controllers, where a single additional CPU clock results in a measurable performance drop.

Still we have to account for 32-bit Coord and 64-bit Coord. I'll work on it.

[Traumflug (mah-jump-ing)]

Googling about rounding found many documents which pretty much agree on that adding a 0.5 is a proper way of rounding, even in corner cases. So I did this with a macro. By using a macro it should work with both, 32-bit and 64-bit coordinates.

Same branch, the pointless commit was replaced with the new one. Thanks for testing and your patience :-)

[DJ Delorie (djdelorie)]

Does adding 0.5 work with negative numbers?

Given that we already have a floating point number that needs to be converted to integer, doing so via lround() is probably the best way.

[Traumflug (mah-jump-ing)]

> Does adding 0.5 work with negative numbers?

Yes. Previously not described completely, the full macro is:

#define DOUBLE_TO_COORD(x) ((x) >= 0 ? (Coord)((x) + 0.5) : (Coord)((x) - 0.5))

[Wiley Hill (wileyhill)]

Maybe gcc quark, try:

Coord unit_to_coord (const Unit *unit, double x)
{
  volatile double base, result;

  if (unit == NULL)
    return -1;

  base = unit->family == METRIC ? MM_TO_COORD (x) : MIL_TO_COORD (x);

  result = base / unit->scale_factor;

  return result;
}