diff options
author | Michael Witten <mfwitten@gmail.com> | 2020-09-29 14:04:22 +0000 |
---|---|---|
committer | Michael Witten <mfwitten@gmail.com> | 2020-09-29 15:47:52 +0000 |
commit | ab3171d21d4034a6524046757a0dc3cef1ae57d1 (patch) | |
tree | 39009937ad5acee71d94ce93361dda38ac74ad10 /Process.h | |
parent | 241e4b3dbf8ebee4a12a337dec725b3547c242e9 (diff) |
Process.{h,c}: Use integer types that are more portable
When building on a 32-bit system, the compiler warned that the
following line uses a constant whose value is the overflow result
of a compile-time computation:
Process.c (line 109): } else if (number < 10000 * ONE_M) {
Namely, this constant expression:
10000 * ONE_M
was intended to produce the following value:
10485760000
However, the result overflowed to produce:
1895825408
The reason for this overflow is as follows:
o The macros are expanded:
10000 * (ONE_K * ONE_K)
10000 * (1024L * 1024L)
o The untyped constant expression "10000" is typed:
10000U * (1024L * 1024L)
o The parenthesized expression is evaluated:
10000U * (1048576L)
o The left operand ("10000U") is converted:
10000L * (1048576L)
Unbound by integer sizes, that last multiplication
would produce the following value:
10485760000
However, on a 32-bit machine, where a long is 32 bits
(really 31 bits when talking about positive numbers),
the maximum value that can be computed is 2**31-1:
2147483647
Consequently, the computation overflows.
o The compiler produces a long int value that is the
the result of overflow (10485760000 % 2**31):
1895825408L
Actually, I think this overflow is implementation-defined,
so it's not even a portable description of what happens.
The solution is to use a long long int (or, even better,
an unsigned long long int) type for the constant expression;
the C standard mandates a sufficiently large maximum value
for such types.
Hence, the following change is made to the bad line:
- } else if (number < 10000 * ONE_M) {
+ } else if (number < 10000ULL * ONE_M) {
However, the whole line is now patently silly, because the
variable "number" is typed "unsigned long", and so it will
always be less than the constant expression (the compiler
will warn about this, too).
Hence, "number" must be typed "unsigned long long"; however,
this necessitates changing all of the string formats from
something like "%lu" to something like "%llu".
Et voila! This commit is born.
Then, for the sake of completeness, the declared types of the
constant-expression macros are updated:
o ONE_K is made unsigned (a "UL" instead of "L")
o ONE_T is computed by introducing "1ULL *"
o Similar changes are made for ONE_DECIMAL_{K,T}
Also, a non-portable overflow-conversion to a signed value
has been replaced with a portable comparison:
- if ((long long) number == -1LL) {
+ if (number == ULLONG_MAX) {
It might be worth reviewing the rest of the code for other
cases where overflows are not handled correctly; even at
runtime, it's often necessary to check for overflow unless
such behavior is expected (especially for signed integer
values, for which overflow has implementation-defined
behavior).
Diffstat (limited to 'Process.h')
-rw-r--r-- | Process.h | 10 |
1 files changed, 5 insertions, 5 deletions
@@ -138,19 +138,19 @@ typedef struct ProcessClass_ { #define Process_sortState(state) ((state) == 'I' ? 0x100 : (state)) -#define ONE_K 1024L +#define ONE_K 1024UL #define ONE_M (ONE_K * ONE_K) #define ONE_G (ONE_M * ONE_K) -#define ONE_T ((long long)ONE_G * ONE_K) +#define ONE_T (1ULL * ONE_G * ONE_K) -#define ONE_DECIMAL_K 1000L +#define ONE_DECIMAL_K 1000UL #define ONE_DECIMAL_M (ONE_DECIMAL_K * ONE_DECIMAL_K) #define ONE_DECIMAL_G (ONE_DECIMAL_M * ONE_DECIMAL_K) -#define ONE_DECIMAL_T ((long long)ONE_DECIMAL_G * ONE_DECIMAL_K) +#define ONE_DECIMAL_T (1ULL * ONE_DECIMAL_G * ONE_DECIMAL_K) void Process_setupColumnWidths(void); -void Process_humanNumber(RichString* str, unsigned long number, bool coloring); +void Process_humanNumber(RichString* str, unsigned long long number, bool coloring); void Process_colorNumber(RichString* str, unsigned long long number, bool coloring); |