On Tue, 10 Apr 2018 15:09:37 +0800 lijiang <lijiang@xxxxxxxxxx> wrote: > 在 2018年04月09日 17:40, Petr Tesarik 写道: >[...] > > 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@xxxxxxxx> > Hi, Petr > It is good that checks the value of "current". > The previous patch resolved system crash in exceptional case. The calculation > result of "calc_delta" may be very large number Hmmm. The resulting ETA is in seconds. To overflow a 32-bit integer, the elapsed time would have to be more than 2^32 seconds, or approx. 136 years. I think it is reasonable to assume that makedumpfile will never run for more than a century... > or even a negative number when making the time jump very great. Yes, the current code will not produce sensible results if the system clock moves backwards. With my patch it will most likely say: ">2day". But it will not work any better if you use a 64-bit integer. Instead, we should be using clock_gettime(CLOCK_MONOTONIC, ...). That's a separate issue that I can solve in another patch. > In this case, it is not enough for "unsigned > integer type" and double. The struct timeval has two 64bit variables in x86 64. First, of the second field (tv_usec), only 20 bits may actually be used, because it can never be greater than one million. Second, to make 100% sure that the target value fits into the eta variable, you would have to consider the worst case: end = ULONG_MAX current = 1 progress = 100.0 / ULONG_MAX delta = { ULONG_MAX, 999999 } eta = ULONG_MAX + 999999/1e6 eta = (100 - progress) * eta / progress Theoretically, this gives: eta = 340345831618257409870852130465035835837 Even a 128-bit integer is too small to hold this maximum theoretical value (129 bits would be needed). Let's stay reasonable. Any value which represents more than a few (dozen) hours is not usable in practice. But hey, to make sure we cannot hit undefined behaviour, why not pass a double to eta_to_human_short()... Going to send an updated patch. Petr T _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec