[PATCH v2] makedumpfile: Use integer arithmetics for the progress bar

Petr Tesarik ptesarik at suse.cz
Tue Apr 10 04:39:29 PDT 2018


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 <ptesarik at suse.com>
---
 print_info.c | 43 ++++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/print_info.c b/print_info.c
index 09e215a..6bfcd11 100644
--- a/print_info.c
+++ b/print_info.c
@@ -16,8 +16,6 @@
 #include "print_info.h"
 #include <time.h>
 #include <string.h>
-#include <stdint.h>
-#include <inttypes.h>
 
 #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.13.6




More information about the kexec mailing list