svn commit: r299064 - head/sys/kern

[Bruce Evans]

On Wed, 4 May 2016, [UTF-8] Roger Pau Monné wrote:

> Log:
>  rtc: fix inverted resolution check
>
>  The current code in clock_register checks if the newly added clock has a
>  resolution value higher than the current one in order to make it the
>  default, which is wrong. Clocks with a lower resolution value should be
>  better than ones with a higher resolution value, in fact with the current
>  code FreeBSD is always selecting the worse clock.
> ...
> Modified: head/sys/kern/subr_rtc.c
> ==============================================================================
> --- head/sys/kern/subr_rtc.c	Wed May  4 12:51:27 2016	(r299063)
> +++ head/sys/kern/subr_rtc.c	Wed May  4 13:48:59 2016	(r299064)
> @@ -84,7 +84,7 @@ clock_register(device_t dev, long res)	/
> {
>
> 	if (clock_dev != NULL) {
> -		if (clock_res > res) {
> +		if (clock_res <= res) {
> 			if (bootverbose)
> 				device_printf(dev, "not installed as "
> 				    "time-of-day clock: clock %s has higher "

This and the next message are still sort of backwards, and have an off-by-1
error.  It is not incorrect for them to say that the current clock has
higher resolution, except for the off-by-1 error.  Higher resolution means
numerically lower and this now matches the code.  But it is confusing.  It
is better to say that the current clock has finer resolution.  The off by
1 error is that the current clock is actually also preferred if it has
the same resolution.  The wording "finer or equal" is not so good, and
neither is "not coarser"

Other bugs in these messages:
- the first 2 are are obfuscated by splitting them across 3 lines; the
   third one is only across 2 lines
- I think they are misformatted (too long) in the output too
- the first message says "not installed", but this function is named
   clock_register() and third message says it registers, not installs
- "removed" in the second message is inconsistent with both "registered"
   and "installed".

Other bugs in the printf()s:
- tv_nsec has type long.  %09ld format handles this perfectly, but %09jd
   is used.  This requires more verboseness to cast to intmax_t
- though %09ld handles nanoseconds perfectly, it is a bogus format since
   the resolution is only microseconds.
- casting tv_sec to intmax_t to print it is excessive.  long works on
   general time_t values until 2038 and is used a lot elsewhere in kern,
   and here the value is an adjustment that is known to be small.  In
   fact it is 'long res' divided by 2 million, so it is at least 2
   million times smaller than needed to print it using %ld.

Bruce