Fix numbers types

[wismill]

WIP, made public only to make you aware I am working on this.

Follow-up of #604 that is mostly a refactor of the parser for integers. Will probably split the minor refactoring in another PR.

Fixes #594

[bluetech]

Some possible followups:

• In xkbcomp/scanner we should change the strtoul to strtol since we're using int64_t. While integer literal cannot be negative, the cast of unsigned long to int64_t is lossy. (We could change ival to unsigned but then it just defers the cast so I think we shouldn't).
• We should probably check the add/sub/mul in expr.c for overflows/underflows and error if it happens. C23 has stdckdint.h, maybe we can use some backport of it.

I will try to do this unless you feel like it :)

[wismill]

• In xkbcomp/scanner we should change the strtoul to strtol since we're using int64_t. While integer literal cannot be negative, the cast of unsigned long to int64_t is lossy. (We could change ival to unsigned but then it just defers the cast so I think we shouldn't).

@bluetech I agree, it showed multiple times already with clang-tidy. But it’s not ideal, as strtol returns a long but we want an int64_t.

• We should probably check the add/sub/mul in expr.c for overflows/underflows and error if it happens. C23 has stdckdint.h, maybe we can use some backport of it.

While I agree that would make our implementation more robust:

1. I think I have never seen those in real use.
2. We already use 64 bits ints, although all our ints are max 32 bits. So we already have some great margin.

Do you have a realistic use case in mind?

[bluetech]

@bluetech I agree, it showed multiple times already with clang-tidy. But it’s not ideal, as strtol returns a long but we want an int64_t.

We can use strtoll then I suppose.

I think I have never seen those in real use.

It's new in C23 so probably no one uses it yet, instead using gcc builtins or custom code.

We already use 64 bits ints, although all our ints are max 32 bits. So we already have some great margin.

Do you mean before or after this PR? I think that before, we are using int for both operands and result, so overflow is possible, and after, we are using int64_t for both operands and result, so overflow is still possible. Do I have it wrong?

[bluetech]

Looks great! This was very much needed.

[wismill]

I think I have never seen those in real use.

It's new in C23 so probably no one uses it yet, instead using gcc builtins or custom code.

@bluetech I meant the int operations in XKB files, not the C23 functions.

Do you mean before or after this PR? I think that before, we are using int for both operands and result, so overflow is possible, and after, we are using int64_t for both operands and result, so overflow is still possible. Do I have it wrong?

You are right, but what I mean is that we do not expect the user to input numbers > 32 bits. So after we will handle 32 bits correctly, with some margin to handle overflow and report it. But if the int64_t overflows, then indeed we would not catch it. But I find it very unlikely, see my comment about the use of operations before.

[bluetech]

Oh yes, there is definitely no real use case for doing such things. And the arithmetic isn't even allowed in many places. But I just feel bad about allowing user input to trigger UB (signed int overflow). We could use -fwrapv but it's not portable.

[wismill]

@bluetech I tackled the strtoul issue and fixed 2 more silent casts for compat group and LED index. Added some tests too.

[bluetech]

The extra changes LGTM

[wismill]

Latch batch for this PR. There are still cast warnings, but I leave it for a further PR.

[wismill]

Note to self: check the Windows CI logs for further cast warnings!