NetBSD Problem Report #36550

From woods@building.weird.com  Sun Jun 24 18:55:18 2007
Return-Path: <woods@building.weird.com>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id E0A6363B882
	for <gnats-bugs@gnats.netbsd.org>; Sun, 24 Jun 2007 18:55:18 +0000 (UTC)
Message-Id: <m1I2XEy-002IwcC@building.weird.com>
Date: Sun, 24 Jun 2007 14:55:16 -0400 (EDT)
From: "Greg A. Woods" <woods@planix.com>
Sender: "Greg A. Woods" <woods@building.weird.com>
Reply-To: "Greg A. Woods" <woods@planix.com>
To: gnats-bugs@NetBSD.org
Subject: some strong suggestions for improvements on share/misc/style
X-Send-Pr-Version: 3.95

>Number:         36550
>Category:       standards
>Synopsis:       some strong suggestions for improvements on share/misc/style
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    standards-manager
>State:          closed
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sun Jun 24 19:00:00 +0000 2007
>Closed-Date:    Sun Apr 10 15:48:17 +0000 2011
>Last-Modified:  Sun Apr 10 20:15:02 +0000 2011
>Originator:     Greg A. Woods
>Release:        netbsd-current
>Organization:
Planix, Inc.; Toronto, Ontario; Canada
>Environment:
>Description:

	The NetBSD "knf" style is quite good, but it falls down very
	badly in a number of relatively minor places.

	The enclosed changes should speak for themselves.  They're based
	on my own 25 years of experience doing intensive C programming,
	with the majority of my time spent on modifying other people's
	code.

	Please consider them as very strong suggestions for improving
	the readability and _reliability_ of NetBSD code.

	Note also that, as it happens, many of these changes work very
	well with the following settings for GNU Emacs c-mode, which,
	for example, makes formatting continued lines and comment
	paragraphs a one-button job.  You guys using just "vi" will get
	carpal tunnel syndrome trying to do the same re-formatting
	manually.  :-)

;; the following isn't 100% complete (e.g. it makes use of private
;; functions defined elsewhere in my ~/.emacs.el).  Contact me for a
;; complete working exmaple, or fetch it from the usual place (i.e. my
;; own directory on my ftp server)
;;
(require 'cc-mode)

(setq c-font-lock-extra-types
      '("FILE"
	"fd_set"
	"jmp_buf"
	"va_list"
	"\\sw+_t"
	"t_\\sw+"
	"u_\\sw+"
	"uchar"
	"uint"
	"ulong"
	"unchar"
	"ushort"))

(defconst my-c-style
  '((c-backslash-column . 78)
    (c-basic-offset . 8)
    (c-block-comment-prefix . "* ")
    (c-cleanup-list . (brace-else-brace
		       brace-elseif-brace
		       scope-operator)) ; (scope-operator)
    (c-comment-continuation-stars . "* ")
    (c-comment-only-line-offset . (0 . 0))
    ;; ACTION can be either a function symbol or a list containing any
    ;; combination of the symbols `before' or `after'.  If the list is empty,
    ;; no newlines are inserted either before or after the brace.
    (c-hanging-braces-alist . ((block-open . (after))
			       (block-close . (before))
			       (brace-list-open . nil)
			       (brace-list-close . nil)
			       (brace-list-intro . nil)
			       (brace-list-entry . nil)
			       (class-open . (after))
			       (class-close . nil)
			       (defun-open . (before after))
			       (defun-close . (before))
			       (inline-open . nil)
			       (inline-close . nil)
			       (statement-case-open . nil)
			       (substatement-open . nil)))
    (c-hanging-colons-alist . ((member-init-intro before)
			       (inher-intro)
			       (case-label after)
			       (label after)
			       (access-label after)))
    (c-label-minimum-indentation . 0)
    ;; an OFFSET is nil; an inteter (usually zero); one of the symbols:  `+',
    ;; `-', `++', `--', `*', or `/' (a positive or negative multiple of
    ;; `c-basic-offset' is added; 1, -1, 2, -2, 0.5, and -0.5, respectively); a
    ;; vector; a function; or a list.
    (c-offsets-alist . ((arglist-close . c-lineup-close-paren) ; +
			(arglist-cont-nonempty . c-lineup-arglist) ; +
			(arglist-intro . c-lineup-arglist-intro-after-paren) ; +
			(block-open . -) ; 0
			(func-decl-cont . 0) ; +
			(inline-open . 0) ; +
                        (statement-case-open . *) ; 0
			(statement-cont . c-lineup-math) ; +
			(substatement-open . 0)))) ; +
  "My PERSONAL C Style, similar to NetBSD KNF.")
(c-add-style "PERSONAL" my-c-style nil)

(defun my-c-mode-common-hook ()
  "My setup hook to be called by all CC Mode modes for common initializations."

  ;; other customizations
  (setq tab-width 8)			; normal, standard, default TAB chars
  (setq fill-column 79)
  (setq comment-column 40)
  (eval-when-compile
    (if (< init-emacs-type 21)
	(defvar comment-style)))
  (setq comment-style 'extra-line)	; not used, but maybe someday?
  (setq indent-tabs-mode t)		; only use tabs

  (if (elisp-file-in-loadpath-p "filladapt")
      (progn
	(require 'filladapt)
	(c-setup-filladapt)
	;; we supposedly can't autoload this thing, yet that means this
	;; function will not be defined at compile time...
	(eval-when-compile
	  (if (elisp-file-in-loadpath-p "filladapt")
	      (require 'filladapt)))
	(turn-on-filladapt-mode)))

  ;; CC Mode things that are not style variables...
  (setq c-echo-syntactic-information-p nil)
  (setq c-electric-pound-behavior '(alignleft)) ; nil
  (setq c-recognize-knr-p t)		; yes, PLEASE!
  (setq c-tab-always-indent nil)	; insert tabs if not in left margin

  (c-toggle-auto-state 1)		; try this on for size!

  ;; keybindings for all of the supported languages.  We can put these in
  ;; c-mode-base-map because awk-mode-map, c-mode-map, c++-mode-map,
  ;; objc-mode-map, java-mode-map, idl-mode-map, pike-mode-map, and so on
  ;; inherit from it.
  (define-key c-mode-base-map "\C-m" 'c-context-line-break)
  (define-key c-mode-base-map "\ej" 'c-fill-paragraph)

  ;; even cc-mode is sometimes too over-bearing.  It seems to
  ;; insist re-setting some key bindings without regard to the
  ;; global key map.
  (override-local-key-settings)
  (override-default-variable-settings))

(add-hook 'c-mode-common-hook 'my-c-mode-common-hook)

;; Derived modes don't have their major-mode (or mode-name) set until after the
;; parent mode has been initialized.  For example this causes c-default-style
;; to be useless with any modes derived from c-mode.  This silly function
;; attempts to work around that bug and can be used in the initialization hook
;; for any such mode derived from c-mode (such as awk-mode).
;;
(defun my-derived-c-mode-hook ()
  "Silly setup hook to be called by modes derived from c-mode."
  (let ((style (if (stringp c-default-style)
		   c-default-style
		 (or (cdr (assq major-mode c-default-style))
		     (cdr (assq 'other c-default-style))
		     "gnu"))))
    (c-set-style style 't)))


>How-To-Repeat:

>Fix:

	Note that some of these changes are merely re-folding of
	paragraphs....

Index: share/misc/style
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/share/misc/style,v
retrieving revision 1.38
diff -u -r1.38 style
--- share/misc/style	2 Nov 2006 22:44:41 -0000	1.38
+++ share/misc/style	24 Jun 2007 18:26:28 -0000
@@ -1,8 +1,10 @@
+/* -*-c-*- */
 /* $NetBSD: style,v 1.38 2006/11/02 22:44:41 christos Exp $ */

 /*
- * The revision control tag appears first, with a blank line after it.
- * Copyright text appears after the revision control tag.
+ * The revision control tag appears first, possibly preceded by an emacs mode
+ * identifier (which must be on the very first line), with a blank line after
+ * it.  Copyright text appears after the revision control tag.
  */

 /*
@@ -38,26 +40,48 @@
 /* Most single-line comments look like this. */

 /*
- * Multi-line comments look like this.  Make them real sentences.  Fill
- * them so they look like real paragraphs.
+ * Multi-line comments look like this.  Make them real sentences.  Fold them at
+ * 80 characters so they look like real paragraphs.
+ *
+ * Put blank lines between paragraphs.
  */

 /*
- * Attempt to wrap lines longer than 80 characters appropriately.
- * Refer to the examples below for more information.
+ * Attempt to fold all lines longer than 80 characters appropriately.
+ *
+ * The only place this is critically important though is in the paragraphs of
+ * text in a comment.  Don't fold lines where doing so would make readability
+ * worse than requiring the reader to scroll horizontally should they be so
+ * unfortunate to be using a narrow terminal or window.
+ *
+ * Refer to the examples below.
  */

 /*
  * EXAMPLE HEADER FILE:
  *
  * A header file should protect itself against multiple inclusion.
+ *
  * E.g, <sys/socket.h> would contain something like:
+ *
+ * In most case one should indent the preprocessor identifiers after the `#' by
+ * one space for every level of nesting.
  */
 #ifndef _SYS_SOCKET_H_
-#define _SYS_SOCKET_H_
+# define _SYS_SOCKET_H_
+
 /*
  * Contents of #include file go between the #ifndef and the #endif at the end.
+ *
+ * Preprocessor identifiers should not be indented an extra level for the
+ * multiple-inclusion protection wrapper.
  */
+#define	NULL		0
+#if defined(_XOPEN_SOURCE) || defined(_NETBSD_SOURCE)
+# define P_tmpdir	"/var/tmp/"
+#endif
+/* a blank line before the end of the multiple-inclusion wrapper */
+
 #endif /* !_SYS_SOCKET_H_ */
 /*
  * END OF EXAMPLE HEADER FILE.
@@ -70,6 +94,7 @@

 /*
  * If it's a network program, put the network include files next.
+ *
  * Group the includes files by subdirectory.
  */
 #include <net/if.h>
@@ -79,8 +104,9 @@
 #include <protocols/rwhod.h>

 /*
- * Then there's a blank line, followed by the /usr include files.
- * The /usr include files should be sorted!
+ * Then there's a blank line, followed by the /usr/include files.
+ *
+ * The /usr/include files can and should be sorted!
  */
 #include <assert.h>
 #include <errno.h>
@@ -99,12 +125,16 @@

 /*
  * ANSI function declarations for private functions (i.e. functions not used
- * elsewhere) and the main() function go at the top of the source module. 
+ * elsewhere) and the main() function go at the top of the source module.
+ *
  * Don't associate a name with the types.  I.e. use:
+ *
  *	void function(int);
+ *
  * Use your discretion on indenting between the return type and the name, and
  * how to wrap a prototype too long for a single line.  In the latter case,
  * lining up under the initial left parenthesis may be more readable.
+ *
  * In any case, consistency is important!
  */
 static char *function(int, int, float, int);
@@ -115,14 +145,20 @@

 /*
  * Macros are capitalized, parenthesized, and should avoid side-effects.
- * Spacing before and after the macro name may be any whitespace, though
- * use of TABs should be consistent through a file.
- * If they are an inline expansion of a function, the function is defined
- * all in lowercase, the macro has the same name all in uppercase.
+ *
+ * Spacing before and after the macro name may be any whitespace, though use of
+ * TABs should be consistent through a file.
+ *
+ * If they are an inline expansion of a function, the function is defined all
+ * in lowercase, the macro has the same name all in uppercase.
+ *
  * If the macro is an expression, wrap the expression in parenthesis.
+ * 
  * If the macro is more than a single statement, use ``do { ... } while (0)'',
- * so that a trailing semicolon works.  Right-justify the backslashes; it
- * makes it easier to read. The CONSTCOND comment is to satisfy lint(1).
+ * so that a trailing semicolon works.  Right-justify the backslashes; it makes
+ * it easier to see.
+ *
+ * The CONSTCOND comment is to inform lint(1) of the constant test condition.
  */
 #define	MACRO(v, w, x, y)						\
 do {									\
@@ -139,21 +175,34 @@
 } et;

 /*
- * When declaring variables in structures, declare them organized by use in
- * a manner to attempt to minimize memory wastage because of compiler alignment
- * issues, then by size, and then by alphabetical order. E.g, don't use
- * ``int a; char *b; int c; char *d''; use ``int a; int b; char *c; char *d''.
+ * When declaring variables in structures, declare them organized by use in a
+ * manner to attempt to minimize memory wastage because of compiler alignment
+ * issues, then by size, and then by alphabetical order. E.g, don't use:
+ *
+ *	int a;
+ *	char *b;
+ *	int c;
+ *	char *d;
+ *
+ * use:
+ *
+ * 	int a;
+ * 	int b;
+ * 	char *c;
+ * 	char *d;
+ *
  * Each variable gets its own type and line, although an exception can be made
  * when declaring bitfields (to clarify that it's part of the one bitfield).
+ *
  * Note that the use of bitfields in general is discouraged.
  *
- * Major structures should be declared at the top of the file in which they
- * are used, or in separate header files, if they are used in multiple
- * source files.  Use of the structures should be by separate declarations
- * and should be "extern" if they are declared in a header file.
+ * Major structures should be declared at the top of the file in which they are
+ * used, or in separate header files, if they are used in multiple source
+ * files.  Use of the structures should be by separate declarations and should
+ * be "extern" if they are declared in a header file.
  *
- * It may be useful to use a meaningful prefix for each member name.
- * E.g, for ``struct softc'' the prefix could be ``sc_''.
+ * It may be useful to use a meaningful prefix for each member name.  E.g, for
+ * ``struct softc'' the prefix could be ``sc_''.
  */
 struct foo {
 	struct foo *next;	/* List of active foo */
@@ -175,9 +224,8 @@
 uint32_t zero;

 /*
- * All major routines should have a comment briefly describing what
- * they do.  The comment before the "main" routine should describe
- * what the program does.
+ * All major routines should have a comment briefly describing what they do.
+ * The comment before the "main" routine should describe what the program does.
  */
 int
 main(int argc, char *argv[])
@@ -187,22 +235,21 @@
 	char *ep;

 	/*
-	 * At the start of main(), call setprogname() to set the program
-	 * name.  This does nothing on NetBSD, but increases portability
-	 * to other systems.
+	 * At the start of main(), call setprogname() to set the program name.
+	 * This does nothing on NetBSD, but increases portability to other
+	 * systems.
 	 */
 	setprogname(argv[0]);

 	/*
-	 * For consistency, getopt should be used to parse options.
-	 * Options should be sorted in the getopt call and the switch
-	 * statement, unless parts of the switch cascade.  For the
-	 * sorting order, see the usage() example below.  Don't forget
-	 * to add option descriptions to the usage and the manpage.
-	 * Elements in a switch statement that cascade should have a
-	 * FALLTHROUGH comment.  Numerical arguments should be checked
-	 * for accuracy.  Code that cannot be reached should have a
-	 * NOTREACHED comment.
+	 * For consistency, getopt should be used to parse options.  Options
+	 * should be sorted in the getopt call and the switch statement, unless
+	 * parts of the switch cascade.  For the sorting order, see the usage()
+	 * example below.  Don't forget to add option descriptions to the usage
+	 * and the manpage.  Elements in a switch statement that cascade should
+	 * have a FALLTHROUGH comment.  Numerical arguments should be checked
+	 * for accuracy.  Code that cannot be reached should have a NOTREACHED
+	 * comment.
 	 */
 	while ((ch = getopt(argc, argv, "abn")) != -1) {
 		switch (ch) {		/* Indent the switch. */
@@ -215,9 +262,16 @@
 		case 'n':
 			errno = 0;
 			num = strtol(optarg, &ep, 10);
+			/*
+			 * fold long expressions and line up continuation lines
+			 * as appropriate for nesting, with the operators
+			 * trailing...
+			 */
 			if (num <= 0 || *ep != '\0' || (errno == ERANGE &&
-			    (num == LONG_MAX || num == LONG_MIN)) )
+							(num == LONG_MAX ||
+							 num == LONG_MIN))) {
 				errx(1, "illegal number -- %s", optarg);
+			}
 			break;
 		case '?':
 		default:
@@ -230,24 +284,29 @@

 	/*
 	 * Space after keywords (while, for, return, switch).  No braces are
-	 * required for control statements with only a single statement,
-	 * unless it's a long statement.
+	 * required for control statements with only a single statement, unless
+	 * it's a long statement, but always using braces makes for better
+	 * readability, fewer stupid bugs, and more easily navigable code
+	 * (e.g. in your text editor).
 	 *
 	 * Forever loops are done with for's, not while's.
 	 */
-	for (p = buf; *p != '\0'; ++p)
+	for (p = buf; *p != '\0'; ++p) {
 		continue;		/* Explicit no-op */
-	for (;;)
+	}
+	for (;;) {
 		stmt;
+	}

 	/*
-	 * Braces are required for control statements with a single statement
-	 * that may expand to nothing.
+	 * Another reason for always using braces.  They are required for
+	 * control statements with a single statement that may expand to
+	 * nothing.  For example:
 	 */
 #ifdef DEBUG_FOO
-#define DPRINTF(a)
+# define DPRINTF(a)	/* nothing */
 #else
-#define DPRINTF(a) printf a
+# define DPRINTF(a)	printf a
 #endif
 	if (broken) {
 		DPRINTF(("broken is %d\n", broken));
@@ -263,15 +322,13 @@
 	}

 	/* Second level indents are four spaces. */
-	while (cnt < 20)
-		z = a + really + long + statement + that + needs + two lines +
-		    gets + indented + four + spaces + on + the + second +
-		    and + subsequent + lines;
-
+	while (cnt < 20) {
+		z = a + really + longish + statement + that + needs + two +
+		    lines + gets + indented + as + needed + on + the +
+		    second + and + subsequent + lines + line + up;
+	} /* braces are also particularly important for multi-line stmts */
 	/*
 	 * Closing and opening braces go on the same line as the else.
-	 * Don't add braces that aren't necessary except in cases where
-	 * there are ambiguity or readability issues.
 	 */
 	if (test) {
 		/*
@@ -285,18 +342,21 @@
 	} else if (bar) {
 		stmt;
 		stmt;
-	} else
+	} else {
 		stmt;
+	} /* braces after if/elseif/else are particularly important! */

 	/* No spaces after function names. */
-	if ((result = function(a1, a2, a3, a4)) == NULL)
+	if ((result = function(a1, a2, a3, a4)) == NULL) {
 		exit(1);
+	}

 	/*
-	 * Unary operators don't require spaces, binary operators do.
-	 * Don't excessively use parenthesis, but they should be used if
-	 * statement is really confusing without them, such as:
-	 * a = b->c[0] + ~d == (e || f) || g && h ? i : j >> 1;
+	 * Unary operators don't require spaces, binary operators do.  Don't
+	 * excessively use parenthesis, but they should be used if statement is
+	 * really confusing without them, such as:
+	 *
+	 *	a = b->c[0] + ~d == (e || f) || g && h ? i : j >> 1;
 	 */
 	a = ((b->c[0] + ~d == (e || f)) || (g && h)) ? i : (j >> 1);
 	k = !(l & FLAGS);
@@ -304,27 +364,35 @@
 	/*
 	 * Exits should be EXIT_SUCCESS on success, and EXIT_FAILURE on
 	 * failure.  Don't denote all the possible exit points, using the
-	 * integers 1 through 127.  Avoid obvious comments such as "Exit
-	 * 0 on success.". Since main is a function that returns an int,
-	 * prefer returning from it, than calling exit.
+	 * integers 1 through 127.  Avoid obvious comments such as "Exit 0 on
+	 * success.". Since main is a function that returns an int, prefer
+	 * returning from it, than calling exit.
 	 */
-	return EXIT_SUCCESS;
+	return (EXIT_SUCCESS);
 }

 /*
- * The function type must be declared on a line by itself
- * preceding the function.
+ * The function type must be declared on a line by itself preceding the
+ * function and the function name must start at the beginning of the line.
+ *
+ * The function parameters should be declared on separate lines, indented by
+ * tabs, to facilitate short comments about their purpose or meaning.
+ *
+ * Function prototypes should go in the include file "extern.h".
  */
 static char *
-function(int a1, int a2, float fl, int a4)
+function(
+	int a1,				/* the first and best parameter */
+	int a2,				/* a similar parameter */
+	float fl,			/* what a number that will be! */
+	int a4)				/* and lastly a final parameter */
 {
 	/*
-	 * When declaring variables in functions declare them sorted by size,
-	 * then in alphabetical order; multiple ones per line are okay.
-	 * Function prototypes should go in the include file "extern.h".
-	 * If a line overflows reuse the type keyword.
+	 * When defining local variables in functions declare them sorted by
+	 * size, then in alphabetical order; multiple ones per line are okay.
+	 * If a line overflows reuse the type keyword again on a new line.
 	 *
-	 * DO NOT initialize variables in the declarations.
+	 * Use separate lines to initialize variables in definitions.
 	 */
 	extern u_char one;
 	extern char two;
@@ -332,44 +400,60 @@
 	double five;
 	int *six, seven;
 	char *eight, *nine, ten, eleven, twelve, thirteen;
+	char *seventeen = "foo";
+	char *eightteen = "fie";
+	char *nineteen = "fee";
+	char *twenty = "fo";
+	char *twentyone = "fum";
 	char fourteen, fifteen, sixteen;

 	/*
-	 * Casts and sizeof's are not followed by a space.  NULL is any
-	 * pointer type, and doesn't need to be cast, so use NULL instead
-	 * of (struct foo *)0 or (struct foo *)NULL.  Also, test pointers
-	 * against NULL.  I.e. use:
-	 *
-	 *	(p = f()) == NULL
-	 * not:
-	 *	!(p = f())
+	 * Casts and sizeof's are _always_ followed by a space (so as to avoid
+	 * looking like a call to a function pointer).  NULL is any pointer
+	 * type, and does not need to be cast in an assignment or a comparison
+	 * (but do cast it when it's used as a parameter so that it's clear you
+	 * know you're passing a null of the pointer of the given type)
+	 *
+	 * You can test pointers against NULL or truth as desired.  I.e.:
 	 *
-	 * Don't use `!' for tests unless it's a boolean.
-	 * E.g. use "if (*p == '\0')", not "if (!*p)".
+	 *	((p = f()) == NULL)
+	 * or:
+	 *	(!(p = f()))
 	 *
-	 * Routines returning ``void *'' should not have their return
-	 * values cast to more specific pointer types.
+	 * You can use `!' for tests where the idiom suits.  E.g. use either
+	 * "if (*p == '\0')", or "if (!*p)".
+	 *
+	 * Routines returning ``void *'' need not have their return values cast
+	 * to more specific pointer types.
 	 *
 	 * Use err/warn(3), don't roll your own!
 	 */
-	if ((four = malloc(sizeof(struct foo))) == NULL)
-		err(1, NULL);
-	if ((six = (int *)overflow()) == NULL)
+	if ((four = malloc(sizeof(struct foo))) == NULL) {
+		err(1, (char *) NULL);
+	}
+	if ((six = (int *) overflow()) == NULL) {
 		errx(1, "Number overflowed.");
+	}

-	/* No parentheses are needed around the return value. */
-	return eight;
+	/* Parentheses should be used around the return value. */
+	return (eight);
 }

 /*
- * Use ANSI function declarations.  ANSI function braces look like
- * old-style (K&R) function braces.
- * As per the wrapped prototypes, use your discretion on how to format
- * the subsequent lines.
+ * Use ANSI function declarations.  ANSI function braces look like old-style
+ * (K&R) function braces.
+ *
+ * As per the wrapped prototypes, use your discretion on how to format the
+ * subsequent lines.
  */
 static int
-dirinfo(const char *p, struct stat *sb, struct dirent *de, struct statfs *sf,
-	int *rargc, char **rargv[])
+dirinfo(
+	const char *p,			/* a pointer to a constant string */
+	struct stat *sb,		/* some stuff */
+	struct dirent *de,		/* good info */
+	struct statfs *sf,		/* big and important */
+	int *rargc,			/* a return value */
+	char **rargv[])			/* another return value */
 {	/* Insert an empty line if the function has no local variables. */

 	/*
@@ -379,29 +463,41 @@
 	_DIAGASSERT(p != NULL);
 	_DIAGASSERT(filedesc != -1);

-	if (stat(p, sb) < 0)
+	if (stat(p, sb) < 0) {
 		err(1, "Unable to stat %s", p);
+	}

 	/*
 	 * To printf quantities that might be larger that "long", include
 	 * <inttypes.h>, cast quantities to intmax_t or uintmax_t and use
 	 * PRI?MAX constants, which may be found in <machine/int_fmtio.h>.
+	 *
+	 * Break long parameter lists, after long strings if appropriate, and
+	 * line up the continuation lines at the beginning of the first
+	 * parameter.
 	 */
-	(void)printf("The size of %s is %" PRIdMAX " (%#" PRIxMAX ")\n", p,
-	    (intmax_t)sb->st_size, (uintmax_t)sb->st_size);
+	(void) printf("The size of %s is %" PRIdMAX " (%#" PRIxMAX ") and this is a really long string\n",
+		      p, (intmax_t) sb->st_size, (uintmax_t) sb->st_size);

 	/*
 	 * To printf quantities of known bit-width, use the corresponding
 	 * defines (generally only done within NetBSD for quantities that
 	 * exceed 32-bits).
-	 */
-	(void)printf("%s uses %" PRId64 " blocks and has flags %#" PRIx32 "\n",
-	    p, sb->st_blocks, sb->st_flags);
+	 *
+	 * An obvious place to use STD-C string constant concatentation is when
+	 * a single string constant contains multiple lines.  Fold at the
+	 * newlines, indent to match the string above.
+	 */
+	(void) printf("%s uses %" PRId64 " blocks and has flags %#" PRIx32 "\n"
+		      "and this is a second line from the same printf\n",
+		      p, (int64_t) sb->st_blocks, (uint32_t) sb->st_flags);

 	/*
 	 * There are similar constants that should be used with the *scanf(3)
 	 * family of functions: SCN?MAX, SCN?64, etc.
 	 */
+
+	return (0);
 }

 /*
@@ -412,14 +508,16 @@
 #include <stdarg.h>

 void
-vaf(const char *fmt, ...)
+vaf(
+	const char *fmt,		/* the message format */
+	...)				/* a variable parameter list */
 {
 	va_list ap;

 	va_start(ap, fmt);
 	STUFF;
 	va_end(ap);	
-				/* No return needed for void functions. */
+				/* No "return" needed for void functions. */
 }

 static void
@@ -428,26 +526,30 @@

 	/*
 	 * Use printf(3), not fputs/puts/putchar/whatever, it's faster and
-	 * usually cleaner, not to mention avoiding stupid bugs.
-	 * Use snprintf(3) or strlcpy(3)/strlcat(3) instead of sprintf(3);
-	 * again to avoid stupid bugs.
-	 *
-	 * Usage statements should look like the manual pages.
-	 * Options w/o operands come first, in alphabetical order
-	 * inside a single set of braces, upper case before lower case
-	 * (AaBbCc...).  Next are options with operands, in the same
-	 * order, each in braces.  Then required arguments in the
-	 * order they are specified, followed by optional arguments in
-	 * the order they are specified.  A bar (`|') separates
-	 * either/or options/arguments, and multiple options/arguments
-	 * which are specified together are placed in a single set of
-	 * braces.
+	 * usually cleaner, not to mention avoiding stupid bugs.  Use
+	 * snprintf(3) or strlcpy(3)/strlcat(3) instead of sprintf(3); again to
+	 * avoid stupid bugs.
+	 *
+	 * Usage statements should look like in the manual pages.
+	 *
+	 * Options w/o operands come first, in alphabetical order inside a
+	 * single set of braces, upper case before lower case (AaBbCc...).
+	 * Next are options with operands, in the same order, each in braces.
+	 * Then required arguments in the order they are specified, followed by
+	 * optional arguments in the order they are specified.  A bar (`|')
+	 * separates either/or options/arguments, and multiple
+	 * options/arguments which are specified together are placed in a
+	 * single set of braces.
 	 *
 	 * Use getprogname() instead of hardcoding the program name.
 	 *
+	 * The word "usage" should have all lower case letters.
+	 *
 	 * "usage: f [-aDde] [-b b_arg] [-m m_arg] req1 req2 [opt1 [opt2]]\n"
 	 * "usage: f [-a | -b] [-c [-de] [-n number]]\n"
 	 */
-	(void)fprintf(stderr, "usage: %s [-ab]\n", getprogname());
+	(void) fprintf(stderr, "usage: %s [-ab]\n", getprogname());
+	/* a blank line bofore return or exit() */
+
 	exit(EXIT_FAILURE);
 }

>Release-Note:

>Audit-Trail:
From: Roland Illig <rillig@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: standards/36550
Date: Wed, 02 Jan 2008 02:59:53 +0100

 + Casts and sizeof's are _always_ followed by a space ...
 ...
 + if ((four = malloc(sizeof(struct foo))) == NULL) {

 This is a contradiction.

 Why should the return value be enclosed in parentheses?

 Roland

State-Changed-From-To: open->closed
State-Changed-By: jruoho@NetBSD.org
State-Changed-When: Sun, 10 Apr 2011 15:48:17 +0000
State-Changed-Why:

I will close this as "not a bug". If you want to change the KNF, a mailing
list rather than the bug tracking system should be used.



From: "Greg A. Woods" <woods@planix.com>
To: gnats-bugs@NetBSD.org
Cc: standards-manager@netbsd.org,
 netbsd-bugs@netbsd.org,
 gnats-admin@netbsd.org,
 jruoho@NetBSD.org
Subject: Re: standards/36550 (some strong suggestions for improvements on share/misc/style)
Date: Sun, 10 Apr 2011 12:35:19 -0700

 On 2011-04-10, at 8:48 AM, jruoho@NetBSD.org wrote:

 > Synopsis: some strong suggestions for improvements on share/misc/style
 > 
 > State-Changed-From-To: open->closed
 > State-Changed-By: jruoho@NetBSD.org
 > State-Changed-When: Sun, 10 Apr 2011 15:48:17 +0000
 > State-Changed-Why:
 > 
 > I will close this as "not a bug". If you want to change the KNF, a mailing
 > list rather than the bug tracking system should be used.

 I think someone previously said I should put this in GNATS.

From: Jukka Ruohonen <jruohonen@iki.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: standards/36550 (some strong suggestions for improvements on share/misc/style)
Date: Sun, 10 Apr 2011 23:12:58 +0300

 On Sun, Apr 10, 2011 at 07:40:03PM +0000, Greg A. Woods wrote:
 >  > I will close this as "not a bug". If you want to change the KNF, a mailing
 >  > list rather than the bug tracking system should be used.
 >  
 >  I think someone previously said I should put this in GNATS.

 As this affects everyone in the project, it should be widely discussed. No
 one can unilaterally even evaluate the validity of the suggested changes, and
 therefore this was closed. (I personally disagree with most of suggestions.)
 I don't think that it is plausible to make big changes to KNF at this point.

>Unformatted:

Home	PR Database Search