Re: [PATCH]Re: svn commit: r240119 - head/sys/kern

2012-09-08 Thread Aleksandr Rybalko 
On Sat, 8 Sep 2012 02:02:05 +0300
Aleksandr Rybalko r...@freebsd.org 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 b...@optusnet.com.au 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.cTue Sep  4 23:13:24
   2012  (r240118) +++ head/sys/kern/subr_hints.cTue
   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()).
 
 Yeah, now I found right place for it.
 
  
   #include sys/bus.h
  
  Sorting this correctly woruld be an unrelated fix.
  
 
 Yeah, kind of second sys/types.h 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 r...@ddteam.net
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

[PATCH]Re: svn commit: r240119 - head/sys/kern

2012-09-07 Thread Aleksandr Rybalko 
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 b...@optusnet.com.au 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.cTue
  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()).

Yeah, now I found right place for it.

 
  #include sys/bus.h
 
 Sorting this correctly woruld be an unrelated fix.
 

Yeah, kind of second sys/types.h 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.

Patch:
-
Index: subr_hints.c
===
--- subr_hints.c(revision 240161)
+++ subr_hints.c(working copy)
@@ -28,12 +28,12 @@
 __FBSDID($FreeBSD$);
 
 #include sys/param.h
+#include sys/systm.h
+#include sys/bus.h
 #include sys/lock.h
 #include sys/malloc.h
 #include sys/mutex.h
 #include sys/sysctl.h
-#include sys/systm.h
-#include sys/bus.h
 
 /*
  * Access functions for device resources.
@@ -45,7 +45,7 @@ static char *hintp;
 
 /*
  * Define kern.hintmode sysctl, which only accept value 2, that cause
to
- * switch from Static KENV mode to Dynamic KENV. So systems that have
hints
+ * switch from Static KENV mode to Dynamic KENV.  So systems that have
hints
  * compiled into kernel will be able to see/modify KENV (and hints
too). */
 
@@ -60,21 +60,20 @@ sysctl_hintmode(SYSCTL_HANDLER_ARGS)
cp = kern_envp;
value = hintmode;
 
-   /* Fetch candidate for new hintmode value */
+   /* Fetch candidate for new hintmode value. */
error = sysctl_handle_int(oidp, value, 0, req);
-

svn commit: r240119 - head/sys/kern

2012-09-04 Thread Aleksandr Rybalko 
Author: ray
Date: Tue Sep  4 23:16:55 2012
New Revision: 240119
URL: http://svn.freebsd.org/changeset/base/240119

Log:
  Style fixes.
  
  Suggested by:   mdf
  Approved by:  adrian (menthor)

Modified:
  head/sys/kern/subr_hints.c

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
 #include sys/bus.h
 
 /*
@@ -52,9 +52,9 @@ static char *hintp;
 static int
 sysctl_hintmode(SYSCTL_HANDLER_ARGS)
 {
-   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 */
error = sysctl_handle_int(oidp, value, 0, req);
-   if (error || !req-newptr)
+   if (error || req-newptr == NULL)
return (error);
 
if (value != 2)
@@ -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.
+*/
+   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;
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org

Re: svn commit: r240119 - head/sys/kern

2012-09-04 Thread Bruce Evans 

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
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org