-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Description
Description
We're currently trying to make RIOT 64-bit compatible (#20154, #19890).
While surprisingly few changes are necessary, a lot of changes are due to print formatting. Many of them can be solved with the formatting macros like PRIxPTR
, but size_t
remains problematic. There is a format specifier for (s)size_t
(%z
and %zu
), but it is not always supported.
The current coding convention states:
- When printing a
size_t
- use
%u
and cast the variable to(unsigned)
becausenewlib-nano
does
not support%zu
Here is a table of the relevant types and architectures as well as the sizes for x86-64.
arch | int | long | size_t | intptr_t |
---|---|---|---|---|
C spec | >=16 | >= 32 | >=16 | - (most of the time same as size_t) |
avr8, msp430 | 16 | 32 | 16 | 16 |
esp32, arm32, x86 | 32 | 32 | 32 | 32 |
x86-64 | 32 | 64 | 64 | 64 |
We can see that sizeof(size_t) == sizeof(int)
for all currently used architectures and therefore the current coding convention is sound for these cases. But it breaks for x86-64, as it would lead to an overflow if the value is above 32 bits. For cases where we know if a given variable will always fit into 32 bits, this is fine (and should cover most of the current print statements). But if we cannot be sure that the variable will always fit into 32 bits, for example if it is referencing a memory address, this could lead to incorrect output.
I propose to change the coding convention to something that also covers 64-bit architectures, and go through the code base to implement it. Here are some suggestions:
-
Use
unsigned long
/%lu
instead ofunsigned
/%u
.
This would fix the problem on 64-bit architectures, have no impact on 32-bit architectures, but increase stack usage and runtime on 8-bit architectures. -
Use
PRIuPTR
instead of%u
.
This should work on all architectures, but can be confusing as it effectively names a different type. -
Add custom
PRIuSIZE
,PRIxSIZE
, etc. format specifier macros.
This will work on all architectures, but would be a RIOT-specific format specifier macro. Also, we would have to either create a new header or add it to an existing header (likesys/include/architecture.h
), and that header would have to be included in a quite a few sources.
(Of course, feel free to suggest different header location and/or macro names)
I would really appreciate your feedback on what is the best way to proceed.
And if we agree on a solution, I would also appreciate any support in going through the code base to implement any new formatting.