Date: Wed, 5 Sep 2012 14:18:40 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Aleksandr Rybalko <ray@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r240119 - head/sys/kern Message-ID: <20120905135841.D1053@besplex.bde.org> In-Reply-To: <201209042316.q84NGtuA035023@svn.freebsd.org> References: <201209042316.q84NGtuA035023@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 4 Sep 2012, Aleksandr Rybalko wrote:
> Log:
> Style fixes.
>
> Suggested by: mdf
> Approved by: adrian (menthor)
The following style bugs remain. (The density of style bugs is low
enough for them to be easy to fix.)
> Modified: head/sys/kern/subr_hints.c
> ==============================================================================
> --- head/sys/kern/subr_hints.c Tue Sep 4 23:13:24 2012 (r240118)
> +++ head/sys/kern/subr_hints.c Tue Sep 4 23:16:55 2012 (r240119)
> @@ -31,8 +31,8 @@ __FBSDID("$FreeBSD$");
> #include <sys/lock.h>
> #include <sys/malloc.h>
> #include <sys/mutex.h>
> -#include <sys/systm.h>
> #include <sys/sysctl.h>
> +#include <sys/systm.h>
Sorting this correctly would be an unrelated fix (it is a prerequisite
for most headers, since almost any header may use KASSERT()).
> #include <sys/bus.h>
Sorting this correctly woruld be an unrelated fix.
>
> /*
> @@ -52,9 +52,9 @@ static char *hintp;
Sorting and indenting the static variables would be an unrelated fix.
> static int
> sysctl_hintmode(SYSCTL_HANDLER_ARGS)
A bug in svn diff is visible. The variable declaration is worse than
useless as a header for this block of code.
> {
> - int error, i, from_kenv, value, eqidx;
> const char *cp;
> char *line, *eq;
> + int eqidx, error, from_kenv, i, value;
>
> from_kenv = 0;
> cp = kern_envp;
> @@ -62,7 +62,7 @@ sysctl_hintmode(SYSCTL_HANDLER_ARGS)
>
> /* Fetch candidate for new hintmode value */
Comments (except possibly ones at the right of code) should be real
sentences. This one is missing a ".", unlike all older comments
(not at the right of code) in this file.
> error = sysctl_handle_int(oidp, &value, 0, req);
> - if (error || !req->newptr)
> + if (error || req->newptr == NULL)
> return (error);
>
> if (value != 2)
This still has a boolean test for the non-boolean `error'. Now the older
code sets a bad example in all cases where `error' is tested.
> @@ -73,8 +73,11 @@ sysctl_hintmode(SYSCTL_HANDLER_ARGS)
> switch (hintmode) {
> case 0:
> if (dynamic_kenv) {
> - /* Already here */
> - hintmode = value; /* XXX: Need we switch or not ? */
> + /*
> + * Already here. But assign hintmode to 2, to not
> + * check it in the future.
> + */
Sentence breaks should be 2 spaces, as in all older comments in this
file, starting as usual with the copyright. But outside of the
copyright, the style bug of single-space sentence breaks was avoided
in this file mostly by using the larger style bug of using a new line
for most new sentences.
> + hintmode = 2;
> return (0);
> }
> from_kenv = 1;
> @@ -98,7 +101,7 @@ sysctl_hintmode(SYSCTL_HANDLER_ARGS)
> continue;
> }
> eq = strchr(cp, '=');
> - if (!eq)
> + if (eq == NULL)
> /* Bad hint value */
> continue;
> eqidx = eq - cp;
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120905135841.D1053>