myriadrf / libxtrx

High-level XTRX API library
Apache License 2.0
28 stars 27 forks source link

xtrx_open_string: properly terminate 4096-byte paramstrings #19

Closed dankamongmen closed 3 years ago

dankamongmen commented 4 years ago
sergforce commented 4 years ago

Re: Don't use bare %x for uint64_t. Cast to uintmax_t and use %jx. Casting to uintmax_t isn't a better solution since it's 32 bits on 32-bit platforms. PRIu64 macro is the only valid portable way. long long unsigned int and "%llx" will work as well on all supported platform but it's ugly as well

dankamongmen commented 4 years ago

Re: Don't use bare %x for uint64_t. Cast to uintmax_t and use %jx. Casting to uintmax_t isn't a better solution since it's 32 bits on 32-bit platforms. PRIu64 macro is the only valid portable way. long long unsigned int and "%llx" will work as well on all supported platform but it's ugly as well

I disagree. uintmax_t is defined to be "the largest supported integer type". Furthermore, the maximum value of uintmax_t is UINTMAX_MAX, which is guaranteed to be 2^64-1 or more by the standard.

http://www.cplusplus.com/reference/cstdint/

or straight from the C11 Standard:

7.20.2.5 Limits of greatest-width integer types
1
— minimum value of greatest-width signed integer type
−(2 63 − 1)
INTMAX_MIN
— maximum value of greatest-width signed integer type
2 63 − 1
INTMAX_MAX
— maximum value of greatest-width unsigned integer type
UINTMAX_MAX 2 64 − 1

I've attached the relevant pages. 2019-10-07-063450_1911x2158_scrot

chemeris commented 4 years ago

Why can't we just use PRIu64 which is the most straightforward way?

dankamongmen commented 4 years ago

Why can't we just use PRIu64 which is the most straightforward way?

We can certainly do that. Would you like me to make the change and rebase?

chemeris commented 4 years ago

Thanks. That would be appreciated.

dankamongmen commented 4 years ago

The merge brought in a different solution anyway, so it's down to just the string fix.