core/macros: fix SIGNOF() macro when applied to size_t

[benpicco]

Contribution description

This is a strange one, for some reason when doing

size_t foo = 23;
return SIGNOF(foo);

none of the _Generics would match and we get

core/include/macros/math.h:34:62: error: comparison is always false due to limited range of data type [-Werror=type-limits]
   34 |                               default: (long long)(a) < 0 ? -1 : 1)
      |                                                       ^

Testing procedure

The updated unit test should compile.

Issues/PRs references

[kfessel]

shouldn't this be "signed long long" ?

[benpicco]

I thought so too, but when I would do that I'd still get the error.

[maribu]

Could you handle long long as special case to avoid loosing precision on the default? Otherwise we may end up with false results for 64 bit numbers.

[kfessel]

                              default: (0LL < ((long long) a)) ? (-1) : (1) )

worked for me no it did not

i checked and found long long to be longer than unsigned long (4 vs 8 bytes)

[benpicco]

hm this will make zero negative signed, but I hope that won't cause any issues…

[kfessel]

no it is wrong i missed turning the < around

[benpicco]

Yea I figured. Still a > 0 will be false and return -1 for a == 0.

But I supposed we could say the sign of 0 is undefined…

[kfessel]

the compiler also fails for uint32_t

[kfessel]

doesn't the _generics system know about typedefs?

[kfessel]

would

static int _signof(long long a){ return (0LL ) <= ( a ) ? (1) : (-1) ;}
#define SIGNOF(a) _Generic(a, unsigned char: 1,          \
                              unsigned short: 1,         \
                              unsigned int: 1,           \
                              unsigned long: 1,          \
                              unsigned long long: 1,     \
                              default: _signof(a) )

work for the usecases?

[kfessel]

but the cast might also lead to wrong result if a is > uint64_max / 2

[benpicco]

No, then it's not longer compile time const in so much that it can be used to statically initialize the size of an array.

[kfessel]

something seems to be realy wrong it does not fail for uint64_t (but the cast may not be valid in that case ( values > uint64_max / 2 will be considered negative))

but if fails for uint32_t, uint16_t

[kfessel]

may be the warning should just be disabled

or document that it only work up to long

[riot-ci]

Murdock results

✔️ PASSED

8f8bb6c tests/unittests: core: add test for SIGNOF(size_t)

Success	Failures	Total	Runtime
1998	0	2000	06m:25s

Artifacts

• Documentation preview

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

[maribu]

ACK, but please address and inline comment and squash before merging

[kfessel]

@benpicco DIV_ROUND without signum

#define DIV_ROUND(a, b) ((a / b  + ((a % b >= (b) / 2) ? 1:0)))