131c4a07fd
- makedumpfile-do-not-print-ETA-if-progress-is-0.patch: Do not print ETA value if current progress is 0 (bsc#1084936). OBS-URL: https://build.opensuse.org/request/show/594834 OBS-URL: https://build.opensuse.org/package/show/Kernel:kdump/makedumpfile?expand=0&rev=121
91 lines
3.4 KiB
Diff
91 lines
3.4 KiB
Diff
From: Petr Tesarik <ptesarik@suse.com>
|
|
Date: Mon, 9 Apr 2018 09:59:46 +0200
|
|
Subject: Do not print ETA value if current progress is 0
|
|
References: bsc#1084936
|
|
Upstream: submitted 2018-04-09
|
|
|
|
Essentially, the estimated remaining time is calculated as:
|
|
|
|
elapsed * (100 - progress) / progress
|
|
|
|
However, print_progress() is also called when progress is 0. The
|
|
result of a floating point division by zero is either NaN (if
|
|
elapsed is zero), or infinity (if the system clock happens to cross
|
|
a second's boundary since reading the start timestamp).
|
|
|
|
The C standard defines only conversion of floating point values
|
|
within the range of the destination integer variable. This means
|
|
that conversion of NaN or infinity to an integer is undefined
|
|
behaviour. Yes, it happens to produce INT_MIN with GCC on major
|
|
platforms...
|
|
|
|
This bug has gone unnoticed, because the very first call to
|
|
print_progress() does not specify a start timestamp (so it cannot
|
|
trigger the bug), and all subsequent calls are rate-limited to one
|
|
per second. As a result, the bug is triggered very rarely.
|
|
|
|
Before commit e5f96e79d69a1d295f19130da00ec6514d28a8ae, the bug also
|
|
caused a buffer overflow. The buffer overflow is mitigated thanks to
|
|
using snprintf() instead of sprintf(), but the program may still
|
|
invoke undefined behaviour.
|
|
|
|
Note that all other changes in the above-mentioned commit were
|
|
ineffective. They merely reduced the precision of the calculation:
|
|
Why would you add delta.tv_usec as a fraction if the fractional part
|
|
is immediately truncated by a converstion to int64_t?
|
|
|
|
Additionally, when the original bug is hit, the output is still
|
|
incorrect, e.g. on my system I get:
|
|
|
|
Copying data : [ 0.0 %] / eta: -9223372036854775808s
|
|
|
|
For that reason, let me revert the changes from commit
|
|
e5f96e79d69a1d295f19130da00ec6514d28a8ae and fix the bug properly,
|
|
i.e. do not calculate ETA if progress is 0.
|
|
|
|
Last but not least, part of the issue was probably caused by the
|
|
wrong assumption that integers < 100 can be interpreted with max 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@suse.com>
|
|
---
|
|
print_info.c | 12 ++++++------
|
|
1 file changed, 6 insertions(+), 6 deletions(-)
|
|
|
|
--- a/print_info.c
|
|
+++ b/print_info.c
|
|
@@ -352,18 +352,18 @@ static void calc_delta(struct timeval *t
|
|
}
|
|
|
|
/* produce less than 12 bytes on msg */
|
|
-static int eta_to_human_short (int secs, char* msg)
|
|
+static int eta_to_human_short (unsigned secs, char* msg)
|
|
{
|
|
strcpy(msg, "eta: ");
|
|
msg += strlen("eta: ");
|
|
if (secs < 100)
|
|
- sprintf(msg, "%ds", secs);
|
|
+ sprintf(msg, "%us", secs);
|
|
else if (secs < 100 * 60)
|
|
- sprintf(msg, "%dm%ds", secs / 60, secs % 60);
|
|
+ sprintf(msg, "%um%us", secs / 60, secs % 60);
|
|
else if (secs < 48 * 3600)
|
|
- sprintf(msg, "%dh%dm", secs / 3600, (secs / 60) % 60);
|
|
+ sprintf(msg, "%uh%um", secs / 3600, (secs / 60) % 60);
|
|
else if (secs < 100 * 86400)
|
|
- sprintf(msg, "%dd%dh", secs / 86400, (secs / 3600) % 24);
|
|
+ sprintf(msg, "%ud%uh", secs / 86400, (secs / 3600) % 24);
|
|
else
|
|
sprintf(msg, ">2day");
|
|
return 0;
|
|
@@ -391,7 +391,7 @@ print_progress(const char *msg, unsigned
|
|
} else
|
|
progress = 100;
|
|
|
|
- if (start != NULL) {
|
|
+ if (start != NULL && current != 0) {
|
|
calc_delta(start, &delta);
|
|
eta = delta.tv_sec + delta.tv_usec / 1e6;
|
|
eta = (100 - progress) * eta / progress;
|