From owner-svn-src-head@FreeBSD.ORG Sat Sep 8 23:04:15 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 7666E106564A for ; Sat, 8 Sep 2012 23:04:15 +0000 (UTC) (envelope-from ray@ddteam.net) Received: from mail-we0-f182.google.com (mail-we0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id F10D58FC0C for ; Sat, 8 Sep 2012 23:04:14 +0000 (UTC) Received: by weyx56 with SMTP id x56so561937wey.13 for ; Sat, 08 Sep 2012 16:04:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:in-reply-to:references:x-mailer :mime-version:content-type:content-transfer-encoding :x-gm-message-state; bh=N8QUA3lpJ+mWfN3eeTGB3hnCMCok71/95SIJALv8i1g=; b=nxaTkzyGtwR0dxUxJ37KfFfmxPRS9AqqbGjkZP4rqO0yUDzsyvwjWmspxxWpFLREih hbSYZVJ5lVf4ioRsouoVuYbwZGwNha37gka1OGLzQiCKbQp+eT42lFGwEbFfNhoB9jvM eIW2fsPG44aYgtRGboJiVuFhVxJXs7+omOHIg6bZuV5xQIRtH05+M60qR9BVUJ7rHZrC QfswMpPvXz/8aEcq4TlvEZwd5hs1q4A01WyxKxLYSZpICdTq5S8UOgnUlIibJE+PwnZG d2JVBp7X6kzivx+XPPLLe3eMtfECGO4FfI5tnJ46ZWndLWCSwAgp1ydowgnTU3RWI5AE IRsg== Received: by 10.216.132.25 with SMTP id n25mr5055529wei.25.1347145453887; Sat, 08 Sep 2012 16:04:13 -0700 (PDT) Received: from rnote.ddteam.net (27-246-133-95.pool.ukrtel.net. [95.133.246.27]) by mx.google.com with ESMTPS id t7sm11299561wix.6.2012.09.08.16.04.11 (version=SSLv3 cipher=OTHER); Sat, 08 Sep 2012 16:04:12 -0700 (PDT) Date: Sun, 9 Sep 2012 02:04:04 +0300 From: Aleksandr Rybalko To: Aleksandr Rybalko Message-Id: <20120909020404.403b0b6e.ray@ddteam.net> In-Reply-To: <20120908020205.fd7f0c7f.ray@freebsd.org> References: <201209042316.q84NGtuA035023@svn.freebsd.org> <20120905135841.D1053@besplex.bde.org> <20120908020205.fd7f0c7f.ray@freebsd.org> X-Mailer: Sylpheed 3.1.2 (GTK+ 2.24.5; amd64-portbld-freebsd9.0) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Gm-Message-State: ALoCoQnWHz3OLxDUp4Yo/l0ZiVohn1Sm7llRfnxmql5bKC9CtOSSDeHHsZ37IlGV4VxBLA2tbrFS Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Bruce Evans Subject: Re: [PATCH]Re: svn commit: r240119 - head/sys/kern X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 08 Sep 2012 23:04:15 -0000 On Sat, 8 Sep 2012 02:02:05 +0300 Aleksandr Rybalko wrote: > Hi Bruce! > > Did not absorb all details of style(9) yet. But I'm working on that :) > > On Wed, 5 Sep 2012 14:18:40 +1000 (EST) > Bruce Evans wrote: > > > 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 > > > #include > > > #include > > > -#include > > > #include > > > +#include > > > > Sorting this correctly would be an unrelated fix (it is a > > prerequisite for most headers, since almost any header may use > > KASSERT()). > > Yeah, now I found right place for it. > > > > > > #include > > > > Sorting this correctly woruld be an unrelated fix. > > > > Yeah, kind of second for kernel. > > > > > > > /* > > > @@ -52,9 +52,9 @@ static char *hintp; > > > > Sorting and indenting the static variables would be an unrelated > > fix. > > I swear, I was not touch hintp. Same with checkmethod and > use_kenv. :) > > > > > > 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. > > > > Did not get it. About which variable you saying? > > > > > > { > > > - 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. > > Fixed. > > > > > > 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. > > Fixed. > > > > > > @@ -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. > > Already start delimiting sentences with double space. When folks last > time arguing about it, I found power to read only about 10 first > mails. :) > > > > > > + 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 > > Thank you very much Bruce! I am understand how much important is > code style, but still on the way to that point. > [skip old patch] http://people.freebsd.org/~ray/subr_hints_style.patch +1 update by mdf@ hint Many Thanks! P.S. Tested - WORKS :) WBW -- Aleksandr Rybalko