On 01/11/2017 04:44 PM, Sami Kerola wrote: > On Sun, 8 Jan 2017, J William Piggott wrote: > >>> @@ -655,7 +655,8 @@ display_time(const bool hclock_valid, struct timeval hwctime) >>> * If anything goes wrong (and many things can), we return return code 10 >>> * and arbitrary *time_p. Otherwise, return code is 0 and *time_p is valid. >>> */ >>> -static int interpret_date_string(const struct hwclock_control *ctl, time_t * const time_p) >>> +static int interpret_date_string(const struct hwclock_control *ctl, >>> + time_t *const time_p) >> >> >> Sorry for nitpicking. I know you didn't write this, but as long as >> you're making a change here, shouldn't it be: const time_t *time_p? > > Nope. That will make pointer value read-only, and that would cause > following error. > > sys-utils/hwclock.c:720:11: error: assignment of read-only location '*time_p' > *time_p = seconds_since_epoch; > > Code we have makes the pointer read-only, but leaves value writeable. > I'm glad Karel built this forum with #include <dumb_question.h> Thanks for the education and patience. >>> @@ -1031,9 +1032,7 @@ calculate_adjustment(const struct hwclock_control *ctl, >>> exact_adjustment = >>> ((double)(systime - last_time)) * factor / (24 * 60 * 60) >>> + not_adjusted; >>> - tdrift_p->tv_sec = (int) exact_adjustment; >>> - if (exact_adjustment < 0) >>> - tdrift_p->tv_sec--; >>> + tdrift_p->tv_sec = (int) floor(exact_adjustment); >> >> Is the cast required now? > > floor(3) will return double, tv_sec is time_t. Yes I should and have > fixed the cast from int to time_t. Isn't the cast implied by assignment and done automatically? Isn't that why the previous mismatch with the (int) cast worked? In the next line of code we assign a double to tv_usec which is a mismatch also. I built your latest branch with and without the cast and the results were binary identical. 8< ------- > ---------------------------------------------------------------- > The following changes since commit 89958178f6d6ebe0944d423feaea66be521fff43: > If mtab support is disabled, disable ro/rw mtab checks (2017-01-10 17:32:32 +0100) > are available in the git repository at: > git://github.com/kerolasa/lelux-utiliteetit.git hwclock-jwp-reviewed > for you to fetch changes up to d3e33557e6794446c712ca264e54fe4b186a17ca: > hwclock: remove --compare option (2017-01-11 21:04:36 +0000) > ---------------------------------------------------------------- > > Thank you JWP for reviews, I hope I did not miss anything but if I did let > me know and I will follow up. Even better if everyone agrees that this is > now good enough to be merged, and we can move forward with other updates > such as adjtimex(8) work. Thanks for all the work, Sami. I've been testing your latest branch and haven't found any problems. I cannot test all of your changes though, for example I don't have libaudit installed. For what it's worth, I think it is ready. -- To unsubscribe from this list: send the line "unsubscribe util-linux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
