svn commit: r211228 - head/sys/kern

[mdf at FreeBSD.org]

On Thu, Aug 12, 2010 at 9:13 AM, Jung-uk Kim <jkim at freebsd.org> wrote:
> Author: jkim
> Date: Thu Aug 12 16:13:24 2010
> New Revision: 211228
> URL: http://svn.freebsd.org/changeset/base/211228
>
> Log:
>  Provide description for 'machdep.disable_rtc_set' sysctl.  Clean up style(9)
>  nits.  Remove a redundant return statement and an unnecessary variable.
>
> Modified:
>  head/sys/kern/subr_rtc.c
>
> Modified: head/sys/kern/subr_rtc.c
> ==============================================================================
> --- head/sys/kern/subr_rtc.c    Thu Aug 12 15:46:15 2010        (r211227)
> +++ head/sys/kern/subr_rtc.c    Thu Aug 12 16:13:24 2010        (r211228)
> @@ -65,8 +65,8 @@ static long clock_res;
>
>  /* XXX: should be kern. now, it's no longer machdep.  */
>  static int disable_rtc_set;
> -SYSCTL_INT(_machdep, OID_AUTO, disable_rtc_set,
> -       CTLFLAG_RW, &disable_rtc_set, 0, "");
> +SYSCTL_INT(_machdep, OID_AUTO, disable_rtc_set, CTLFLAG_RW, &disable_rtc_set,
> +    0, "Disallow adjusting time-of-day clock");
>
>  void
>  clock_register(device_t dev, long res) /* res has units of microseconds */
> @@ -74,26 +74,22 @@ clock_register(device_t dev, long res)      /
>
>        if (clock_dev != NULL) {
>                if (clock_res > res) {
> -                       if (bootverbose) {
> +                       if (bootverbose)
>                                device_printf(dev, "not installed as "
>                                    "time-of-day clock: clock %s has higher "
>                                    "resolution\n", device_get_name(clock_dev));
> -                       }

While the device_printf() is a single statement, it spans multiple
lines.  It seems in this instance that clarity is preserved better by
using braces even though not required by C.

Quoting style(9):

Space after keywords (if, while, for, return, switch).  No braces ('{'
and '}') are used for control statements with zero or only a single
statement unless that statement is more than a single line in which case
they are permitted.

So both styles are accepted, and I would think that changing this is
needless code churn.

Thanks,
matthew

>                        return;
> -               } else {
> -                       if (bootverbose) {
> -                               device_printf(clock_dev, "removed as "
> -                                   "time-of-day clock: clock %s has higher "
> -                                   "resolution\n", device_get_name(dev));
> -                       }
>                }
> +               if (bootverbose)
> +                       device_printf(clock_dev, "removed as "
> +                           "time-of-day clock: clock %s has higher "
> +                           "resolution\n", device_get_name(dev));
>        }
>        clock_dev = dev;
>        clock_res = res;
> -       if (bootverbose) {
> +       if (bootverbose)
>                device_printf(dev, "registered as a time-of-day clock "
>                    "(resolution %ldus)\n", res);
> -       }
>  }
>
>  /*
> @@ -109,7 +105,7 @@ clock_register(device_t dev, long res)      /
>  void
>  inittodr(time_t base)
>  {
> -       struct timespec ref, ts;
> +       struct timespec ts;
>        int error;
>
>        if (clock_dev == NULL) {
> @@ -136,9 +132,9 @@ inittodr(time_t base)
>
>  wrong_time:
>        if (base > 0) {
> -               ref.tv_sec = base;
> -               ref.tv_nsec = 0;
> -               tc_setclock(&ref);
> +               ts.tv_sec = base;
> +               ts.tv_nsec = 0;
> +               tc_setclock(&ts);
>        }
>  }
>
> @@ -157,9 +153,7 @@ resettodr(void)
>        getnanotime(&ts);
>        ts.tv_sec -= utc_offset();
>        /* XXX: We should really set all registered RTCs */
> -       if ((error = CLOCK_SETTIME(clock_dev, &ts)) != 0) {
> +       if ((error = CLOCK_SETTIME(clock_dev, &ts)) != 0)
>                printf("warning: clock_settime failed (%d), time-of-day clock "
>                    "not adjusted to system time\n", error);
> -               return;
> -       }
>  }
>