From c6b79cbc990b3b4933730205f58812fb44b6fcd5 Mon Sep 17 00:00:00 2001 From: Petr Tesarik Date: Tue, 10 Apr 2018 20:39:00 +0900 Subject: [PATCH] [PATCH v2] makedumpfile: Use integer arithmetics for the progress bar Essentially, the estimated remaining time is calculated as: elapsed * (100 - progress) / progress Since the calculation is done with floating point numbers, it had masked a division by zero (if progress is 0), producing a NaN or infinity. The following conversion to int produces INT_MIN with GCC on major platforms, which originally overflowed the eta buffer. This bug was fixed by commit e5f96e79d69a1d295f19130da00ec6514d28a8ae, but conversion of NaN and infinity is undefined behaviour in ISO C, plus the corresponding output is still wrong, e.g.: Copying data : [ 0.0 %] / eta: -9223372036854775808s Most importantly, using the FPU for a progress bar is overkill. Since the progress percentage is reported with one decimal digit following the decimal point, it can be stored as an integer tenths of a percent. Second, the estimated time can be calculated in milliseconds. Up to 49 days can be represented this way even on 32-bit platforms. Note that delta.tv_usec can be ignored in the subtraction, because the resulting eta is printed as seconds, so elapsed microseconds are irrelevant. Last but not least, the original buffer overflow was probably caused by the wrong assumption that integers < 100 can be interpreted with less than 3 ASCII characters, but that's not true for signed integers. To make eta_to_human_short() a bit safer, use an unsigned integer type. Signed-off-by: Petr Tesarik --- print_info.c | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/makedumpfile-1.6.2/print_info.c b/makedumpfile-1.6.2/print_info.c index 09e215a..6bfcd11 100644 --- a/makedumpfile-1.6.2/print_info.c +++ b/makedumpfile-1.6.2/print_info.c @@ -16,8 +16,6 @@ #include "print_info.h" #include #include -#include -#include #define PROGRESS_MAXLEN "50" @@ -354,21 +352,18 @@ static void calc_delta(struct timeval *tv_start, struct timeval *delta) } /* produce less than 12 bytes on msg */ -static int eta_to_human_short (int64_t secs, char* msg, int maxsize) +static int eta_to_human_short (unsigned long secs, char* msg) { strcpy(msg, "eta: "); msg += strlen("eta: "); if (secs < 100) - snprintf(msg, maxsize, "%"PRId64"s", secs); + sprintf(msg, "%lus", secs); else if (secs < 100 * 60) - snprintf(msg, maxsize, "%"PRId64"m""%"PRId64"s", - secs / 60, secs % 60); + sprintf(msg, "%lum%lus", secs / 60, secs % 60); else if (secs < 48 * 3600) - snprintf(msg, maxsize, "%"PRId64"h""%"PRId64"m", - secs / 3600, (secs / 60) % 60); + sprintf(msg, "%luh%lum", secs / 3600, (secs / 60) % 60); else if (secs < 100 * 86400) - snprintf(msg, maxsize, "%"PRId64"d""%"PRId64"h", - secs / 86400, (secs / 3600) % 24); + sprintf(msg, "%lud%luh", secs / 86400, (secs / 3600) % 24); else sprintf(msg, ">2day"); return 0; @@ -378,37 +373,39 @@ static int eta_to_human_short (int64_t secs, char* msg, int maxsize) void print_progress(const char *msg, unsigned long current, unsigned long end, struct timeval *start) { - float progress; + unsigned progress; /* in promilles (tenths of a percent) */ time_t tm; static time_t last_time = 0; static unsigned int lapse = 0; static const char *spinner = "/|\\-"; struct timeval delta; - int64_t eta; - char eta_msg[32] = " "; + unsigned long eta; + char eta_msg[16] = " "; if (current < end) { tm = time(NULL); if (tm - last_time < 1) return; last_time = tm; - progress = (float)current * 100 / end; + progress = current * 1000 / end; } else - progress = 100; + progress = 1000; - if (start != NULL) { + if (start != NULL && progress != 0) { calc_delta(start, &delta); - eta = delta.tv_sec + delta.tv_usec / 1e6; - eta = (100 - progress) * eta / progress; - eta_to_human_short(eta, eta_msg, sizeof(eta_msg)); + eta = 1000 * delta.tv_sec + delta.tv_usec / 1000; + eta = eta / progress - delta.tv_sec; + eta_to_human_short(eta, eta_msg); } if (flag_ignore_r_char) { - PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%5.1f %%] %c %16s\n", - msg, progress, spinner[lapse % 4], eta_msg); + PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3u.%u %%] %c %16s\n", + msg, progress / 10, progress % 10, + spinner[lapse % 4], eta_msg); } else { PROGRESS_MSG("\r"); - PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%5.1f %%] %c %16s", - msg, progress, spinner[lapse % 4], eta_msg); + PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3u.%u %%] %c %16s", + msg, progress / 10, progress % 10, + spinner[lapse % 4], eta_msg); } lapse++; } -- 2.9.5