zeldon/htop
1
0
Fork 0
mirror of https://github.com/xzeldon/htop.git synced 2024-12-23 22:55:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Process.{h,c}: Use integer types that are more portable

Browse Source 
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).
This commit is contained in:
Michael Witten 2020-09-29 14:04:22 +00:00
parent 241e4b3dbf
commit ab3171d21d
2 changed files with 19 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
Process.c
View File

@ -55,7 +55,7 @@ void Process_setupColumnWidths() {
xSnprintf(Process_pidFormat, sizeof(Process_pidFormat), "%%%dd ", digits);
}
void Process_humanNumber(RichString* str, unsigned long number, bool coloring) {
void Process_humanNumber(RichString* str, unsigned long long number, bool coloring) {
char buffer[11];
int len;
@ -71,48 +71,48 @@ void Process_humanNumber(RichString* str, unsigned long number, bool coloring) {
if (number < 1000) {
//Plain number, no markings
len = snprintf(buffer, 10, "%5lu ", number);
len = snprintf(buffer, 10, "%5llu ", number);
RichString_appendn(str, processColor, buffer, len);
} else if (number < 100000) {
//2 digit MB, 3 digit KB
len = snprintf(buffer, 10, "%2lu", number/1000);
len = snprintf(buffer, 10, "%2llu", number/1000);
RichString_appendn(str, processMegabytesColor, buffer, len);
number %= 1000;
len = snprintf(buffer, 10, "%03lu ", number);
len = snprintf(buffer, 10, "%03llu ", number);
RichString_appendn(str, processColor, buffer, len);
} else if (number < 1000 * ONE_K) {
//3 digit MB
number /= ONE_K;
len = snprintf(buffer, 10, "%4luM ", number);
len = snprintf(buffer, 10, "%4lluM ", number);
RichString_appendn(str, processMegabytesColor, buffer, len);
} else if (number < 10000 * ONE_K) {
//1 digit GB, 3 digit MB
number /= ONE_K;
len = snprintf(buffer, 10, "%1lu", number/1000);
len = snprintf(buffer, 10, "%1llu", number/1000);
RichString_appendn(str, processGigabytesColor, buffer, len);
number %= 1000;
len = snprintf(buffer, 10, "%03luM ", number);
len = snprintf(buffer, 10, "%03lluM ", number);
RichString_appendn(str, processMegabytesColor, buffer, len);
} else if (number < 100000 * ONE_K) {
//2 digit GB, 1 digit MB
number /= 100 * ONE_K;
len = snprintf(buffer, 10, "%2lu", number/10);
len = snprintf(buffer, 10, "%2llu", number/10);
RichString_appendn(str, processGigabytesColor, buffer, len);
number %= 10;
len = snprintf(buffer, 10, ".%1luG ", number);
len = snprintf(buffer, 10, ".%1lluG ", number);
RichString_appendn(str, processMegabytesColor, buffer, len);
} else if (number < 1000 * ONE_M) {
//3 digit GB
number /= ONE_M;
len = snprintf(buffer, 10, "%4luG ", number);
len = snprintf(buffer, 10, "%4lluG ", number);
RichString_appendn(str, processGigabytesColor, buffer, len);
} else if (number < 10000 * ONE_M) {
} else if (number < 10000ULL * ONE_M) {
//1 digit TB, 3 digit GB
number /= ONE_M;
len = snprintf(buffer, 10, "%1lu", number/1000);
len = snprintf(buffer, 10, "%1llu", number/1000);
RichString_appendn(str, largeNumberColor, buffer, len);
number %= 1000;
len = snprintf(buffer, 10, "%03luG ", number);
len = snprintf(buffer, 10, "%03lluG ", number);
RichString_appendn(str, processGigabytesColor, buffer, len);
} else {
//2 digit TB and above
@ -134,7 +134,7 @@ void Process_colorNumber(RichString* str, unsigned long long number, bool colori
processShadowColor = CRT_colors[PROCESS];
}
if ((long long) number == -1LL) {
if (number == ULLONG_MAX) {
int len = snprintf(buffer, 13, " no perm ");
RichString_appendn(str, CRT_colors[PROCESS_SHADOW], buffer, len);
} else if (number >= 100000LL * ONE_DECIMAL_T) {

10
Process.h
View File

@ -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);