On Fri, 18 May 2018 08:22:10 +0000 Masaki Tachibana <mas-tachibana@xxxxxxxxxxxxx> wrote: > Hi Petr and Lianbo, Hi Masaki-san, > Sorry for the late reply. No problem. I know how to wait. ;-) > I will merge the patch into V1.6.4. Thank you very much! > > Thanks > Tachibana > > > > -----Original Message----- > > From: Petr Tesarik [mailto:ptesarik@xxxxxxx] > > Sent: Tuesday, April 10, 2018 8:39 PM > > To: lijiang <lijiang@xxxxxxxxxx> > > Cc: Tachibana Masaki() <mas-tachibana@xxxxxxxxxxxxx>; Nakayama Takuya() <tak-nakayama@xxxxxxxxxxxxx>; > > kexec-ml <kexec@xxxxxxxxxxxxxxxxxxx> > > Subject: [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 <ptesarik@xxxxxxxx> > > --- > > 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 > > > > > > _______________________________________________ > kexec mailing list > kexec@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/kexec _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
