Subject: proposed new KNF [was Re: Time to update KNF?]
To: None <tech-kern@netbsd.org>
From: Luke Mewburn <lukem@cs.rmit.edu.au>
List: tech-kern
Date: 01/18/2000 14:43:44
Chris G. Demetriou writes:
> like, HELLO, it's _2000_, c89 has been around for > 10 years.
> I don't understand why people are hanging on to K&R as a coding
> standard now, since it does have some real known issues, and, despite
> what i'm sure some would claim, is not a standard.  8-)


Here's something I've been sitting on for a while.  It's a
revised KNF style guide (share/misc/style).

I've been holding off sending it for a couple of months (was waiting
for the rc.d flamefest to die down ;-)

I've attached the diffs to the -current misc/style at the end.


Toast away...

Luke.

--- cut here --- file: style
/* $NetBSD: style,v 1.11 1999/07/03 21:47:21 abs Exp $ */

/*
 * The revision control tag appears first.
 * Copyright text appears after the revision control tag.
 */

/*
 * Style guide for the NetBSD KNF (Kernel Normal Form).
 *
 *	from: @(#)style	1.12 (Berkeley) 3/18/94
 */
/*
 * An indent(1) profile approximating the style outlined in
 * this document lives in /usr/share/misc/indent.pro.  It is a
 * useful tool to assist in converting code to KNF, but indent(1)
 * output generated using this profile must not be considered to
 * be an authoritative reference.
 */

/*
 * Source code revision control identifiers appear after any copyright
 * text.  Use the appropriate macros from <sys/cdefs.h>.  Usually only one
 * source file per program contains a __COPYRIGHT() section.
 */
#include <sys/cdefs.h>
#ifndef lint
__COPYRIGHT("@(#) Copyright (c) 1999\n\
	The NetBSD Foundation, inc. All rights reserved.\n");
__RCSID("$NetBSD$");
#endif /* not lint */

/*
 * VERY important single-line comments look like this.
 */

/* 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.
 */

/*
 * Kernel include files come first; normally, you'll need <sys/types.h>
 * OR <sys/param.h>, but not both!  <sys/types.h> includes <sys/cdefs.h>,
 * and it's okay to depend on that.
 */
#include <sys/types.h>		/* Non-local includes in brackets. */

/* If it's a network program, put the network include files next. */
#include <net/if.h>
#include <net/if_dl.h>
#include <net/route.h>
#include <netinet/in.h>
#include <protocols/rwhod.h>

/*
 * Then there's a blank line, followed by the /usr include files.
 * The /usr include files should be sorted!
 */
#include <stdio.h>

/*
 * Global pathnames are defined in /usr/include/paths.h.  Pathnames local
 * to the program go in pathnames.h in the local directory.
 */
#include <paths.h>

/* Then, there's a blank line, and the user include files. */
#include "pathnames.h"		/* Local includes in double quotes. */

/*
 * ANSI function declarations for private functions (i.e. functions not used
 * elsewhere) go at the top of the source module.  Only the kernel has a name
 * associated with the types.  I.e. in the kernel use:
 *	void function(int a);
 * in user-land use:
 *	void function(int);
 *
 * Use the __P macro from the include file <sys/cdefs.h> for prototypes 
 * in header files, for compatibility with non-ANSI compilers.  I.e,
 *	void function __P((int));
 */
static char	*function(int, int, float, int);
static void	 usage(void);

/*
 * Macros are capitalized, parenthesized, and should avoid side-effects.
 * 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 needs 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.
 */
#define	MACRO(x, y)	do {						\
	variable = (x) + (y);						\
	(y) += 2;							\
} while (0)

/* Enum types are capitalized. */
enum enumtype {
	ONE,
	TWO
} et;

/*
 * When declaring variables in structures, declare them sorted by use, then
 * by size, and then by alphabetical order.  The first category normally
 * doesn't apply, but there are exceptions.  Each one gets its own line.
 * Attempt to line-up the entries, using appropriate tabs and spaces.
 *
 * 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.
 */
struct foo {
	struct	foo	*next;		/* List of active foo */
	struct	mumble	 amumble;	/* Comment for mumble */
	int		 bar;
};
struct foo *foohead;		/* Head of global foo list */

/* Make the structure name match the typedef. */
typedef struct _bar {
	int	level;
} BAR;

/*
 * 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(argc, argv)
	int argc;
	char *argv[];
{
	extern char	*optarg;
	extern int	 optind;
	long		 num;
	int		 ch;
	char		*ep;

	/*
	 * 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.  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. */
		case 'a':		/* Don't indent the case. */
			aflag = 1;
			/* FALLTHROUGH */
		case 'b':
			bflag = 1;
			break;
		case 'n':
			num = strtol(optarg, &ep, 10);
			if (num <= 0 || *ep != '\0')
				errx(1, "illegal number -- %s", optarg);
			break;
		case '?':
		default:
			usage();
			/* NOTREACHED */
		}
	argc -= optind;
	argv += optind;

	/*
	 * Space after keywords (while, for, return, switch).  No braces are
	 * used for control statements with zero or only a single statement.
	 *
	 * Forever loops are done with for's, not while's.
	 */
	for (p = buf; *p != '\0'; ++p);
	for (;;)
		stmt;

	/*
	 * Parts of a for loop may be left empty.  Don't put declarations
	 * inside blocks unless the routine is unusually complicated.
	 */
	for (; cnt < 15; cnt++) {
		stmt1;
		stmt2;
	}

	/* Second level indents are four spaces. */
	while (cnt < 20)
		z = a + really + long + statment + that + needs + two lines +
		    gets + indented + four + spaces + on + the + second +
		    and + subsequent + lines;

	/*
	 * Closing and opening braces go on the same line as the else.
	 * Don't add braces that aren't necessary.
	 */
	if (test)
		stmt;
	else if (bar) {
		stmt;
		stmt;
	} else
		stmt;

	/* No spaces after function names. */
	if (error = function(a1, a2))
		exit(error);

	/*
	 * Unary operators don't require spaces, binary operators do.  Don't
	 * use parenthesis unless they're required for precedence, or the
	 * 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);

	/*
	 * Exits should be 0 on success, and 1 on failure.  Don't denote
	 * all the possible exit points, using the integers 1 through 300.
	 * Avoid obvious comments such as "Exit 0 on success."
	 */
	exit(0);
}

/*
 * The function type must be declared on a line by itself
 * preceeding the function.
 */
static char *
function(int a1, int a2, float fl, int a4)
{
	/*
	 * When declaring variables in functions declare them sorted by size,
	 * then in alphabetical order; multiple ones per line are okay.
	 * Function declarations (which are ANSI style) should go in the
	 * include file "extern.h".  If a line overflows reuse the type keyword.
	 *
	 * DO NOT initialize variables in the declarations.
	 */
	extern u_char	 one;
	extern char	 two;
	struct foo	 three, *four;
	double		 five;
	int		*six, seven;
	char		*eight, *nine, ten, eleven, twelve, thirteen;
	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())
	 *
	 * Don't use '!' for tests unless it's a boolean. E.g. use
	 * "if (*p == '\0')", not "if (!*p)".
 	 *
	 * Routines returning void * should not have their return values cast
	 * to any pointer type.
	 *
	 * Use err/warn(3), don't roll your own!
	 */
	if ((four = malloc(sizeof(struct foo))) == NULL)
		err(1, NULL);
	if ((six = (int *)overflow()) == NULL)
		errx(1, "Number overflowed.");
	return (eight);
}

/*
 * Use ANSI function declarations.  ANSI function braces look like
 * old-style (K&R) function braces.
 */
void
function(int a1, int a2)
{
	/* ... */
}

/*
 * Functions that support variable numbers of arguments should look like this.
 */
#include <stdarg.h>

void
vaf(const char *fmt, ...)
{
	va_list	ap;

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

static void
usage()
{	/* Insert an empty line if the function has no local variables. */

	/*
	 * Use printf(3), not fputs/puts/putchar/whatever, it's faster and
	 * usually cleaner, not to mention avoiding 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.  Followed by options with operands, in alphabetical order,
	 * each in braces.  Followed by 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.
	 *
	 * "usage: f [-ade] [-b b_arg] [-m m_arg] req1 req2 [opt1 [opt2]]\n"
	 * "usage: f [-a | -b] [-c [-de] [-n number]]\n"
	 */
	(void)fprintf(stderr, "usage: f [-ab]\n");
	exit(1);
}
--- cut here ---


	====================
	= here's the diffs =
	====================

--- cut here --- file: style.diffs
Index: style
===================================================================
RCS file: /cvsroot/sharesrc/share/misc/style,v
retrieving revision 1.11
diff -p -r1.11 style
*** style	1999/07/03 21:47:21	1.11
--- style	2000/01/18 03:40:39
***************
*** 1,7 ****
! /*	$NetBSD: style,v 1.11 1999/07/03 21:47:21 abs Exp $	*/
  
  /*
!  * Style guide for the 4BSD KNF (Kernel Normal Form).
   *
   *	from: @(#)style	1.12 (Berkeley) 3/18/94
   */
--- 1,12 ----
! /* $NetBSD: style,v 1.11 1999/07/03 21:47:21 abs Exp $ */
  
  /*
!  * The revision control tag appears first.
!  * Copyright text appears after the revision control tag.
!  */
! 
! /*
!  * Style guide for the NetBSD KNF (Kernel Normal Form).
   *
   *	from: @(#)style	1.12 (Berkeley) 3/18/94
   */
***************
*** 14,19 ****
--- 19,36 ----
   */
  
  /*
+  * Source code revision control identifiers appear after any copyright
+  * text.  Use the appropriate macros from <sys/cdefs.h>.  Usually only one
+  * source file per program contains a __COPYRIGHT() section.
+  */
+ #include <sys/cdefs.h>
+ #ifndef lint
+ __COPYRIGHT("@(#) Copyright (c) 1999\n\
+ 	The NetBSD Foundation, inc. All rights reserved.\n");
+ __RCSID("$NetBSD$");
+ #endif /* not lint */
+ 
+ /*
   * VERY important single-line comments look like this.
   */
  
***************
*** 51,93 ****
  #include <paths.h>
  
  /* Then, there's a blank line, and the user include files. */
! #include "pathnames.h"		/* Local includes in double quotes. */		
  
  /*
   * ANSI function declarations for private functions (i.e. functions not used
!  * elsewhere) go at the top of the source module.  Use the __P macro from
!  * the include file <sys/cdefs.h>.  Only the kernel has a name associated with
!  * the types, i.e. in the kernel use:
!  *
!  *	void function __P((int a));
   *
!  * in user land use:
!  *
   *	void function __P((int));
   */
! static char	*function __P((int, const char *));
! static void	 usage __P((void));
  
  /*
   * Macros are capitalized, parenthesized, and should avoid side-effects.
   * 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 needs more than a single line, use braces.  Right-justify the
!  * backslashes, it makes it easier to read.
   */
! #define	MACRO(x, y) {							\
  	variable = (x) + (y);						\
  	(y) += 2;							\
! }
  
  /* Enum types are capitalized. */
! enum enumtype { ONE, TWO } et;
  
  /*
   * When declaring variables in structures, declare them sorted by use, then
   * by size, and then by alphabetical order.  The first category normally
   * doesn't apply, but there are exceptions.  Each one gets its own line.
!  * Put a tab after the first word, i.e. use "int^Ix;" and "struct^Ifoo *x;".
   *
   * 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
--- 68,114 ----
  #include <paths.h>
  
  /* Then, there's a blank line, and the user include files. */
! #include "pathnames.h"		/* Local includes in double quotes. */
  
  /*
   * ANSI function declarations for private functions (i.e. functions not used
!  * elsewhere) go at the top of the source module.  Only the kernel has a name
!  * associated with the types.  I.e. in the kernel use:
!  *	void function(int a);
!  * in user-land use:
!  *	void function(int);
   *
!  * Use the __P macro from the include file <sys/cdefs.h> for prototypes 
!  * in header files, for compatibility with non-ANSI compilers.  I.e,
   *	void function __P((int));
   */
! static char	*function(int, int, float, int);
! static void	 usage(void);
  
  /*
   * Macros are capitalized, parenthesized, and should avoid side-effects.
   * 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 needs 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.
   */
! #define	MACRO(x, y)	do {						\
  	variable = (x) + (y);						\
  	(y) += 2;							\
! } while (0)
  
  /* Enum types are capitalized. */
! enum enumtype {
! 	ONE,
! 	TWO
! } et;
  
  /*
   * When declaring variables in structures, declare them sorted by use, then
   * by size, and then by alphabetical order.  The first category normally
   * doesn't apply, but there are exceptions.  Each one gets its own line.
!  * Attempt to line-up the entries, using appropriate tabs and spaces.
   *
   * 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
*************** enum enumtype { ONE, TWO } et;
*** 95,103 ****
   * and should be "extern" if they are declared in a header file.
   */
  struct foo {
! 	struct	foo *next;	/* List of active foo */
! 	struct	mumble amumble;	/* Comment for mumble */
! 	int	bar;
  };
  struct foo *foohead;		/* Head of global foo list */
  
--- 116,124 ----
   * and should be "extern" if they are declared in a header file.
   */
  struct foo {
! 	struct	foo	*next;		/* List of active foo */
! 	struct	mumble	 amumble;	/* Comment for mumble */
! 	int		 bar;
  };
  struct foo *foohead;		/* Head of global foo list */
  
*************** struct foo *foohead;		/* Head of global 
*** 105,111 ****
  typedef struct _bar {
  	int	level;
  } BAR;
! 	
  /*
   * All major routines should have a comment briefly describing what
   * they do.  The comment before the "main" routine should describe
--- 126,132 ----
  typedef struct _bar {
  	int	level;
  } BAR;
! 
  /*
   * All major routines should have a comment briefly describing what
   * they do.  The comment before the "main" routine should describe
*************** main(argc, argv)
*** 116,126 ****
  	int argc;
  	char *argv[];
  {
! 	extern char *optarg;
! 	extern int optind;
! 	long num;
! 	int ch;
! 	char *ep;
  
  	/*
  	 * For consistency, getopt should be used to parse options.  Options
--- 137,147 ----
  	int argc;
  	char *argv[];
  {
! 	extern char	*optarg;
! 	extern int	 optind;
! 	long		 num;
! 	int		 ch;
! 	char		*ep;
  
  	/*
  	 * For consistency, getopt should be used to parse options.  Options
*************** main(argc, argv)
*** 140,147 ****
  			break;
  		case 'n':
  			num = strtol(optarg, &ep, 10);
!                         if (num <= 0 || *ep != '\0')
!                                 err(1,"illegal number -- %s", optarg);
  			break;
  		case '?':
  		default:
--- 161,168 ----
  			break;
  		case 'n':
  			num = strtol(optarg, &ep, 10);
! 			if (num <= 0 || *ep != '\0')
! 				errx(1, "illegal number -- %s", optarg);
  			break;
  		case '?':
  		default:
*************** main(argc, argv)
*** 160,166 ****
  	for (p = buf; *p != '\0'; ++p);
  	for (;;)
  		stmt;
! 	
  	/*
  	 * Parts of a for loop may be left empty.  Don't put declarations
  	 * inside blocks unless the routine is unusually complicated.
--- 181,187 ----
  	for (p = buf; *p != '\0'; ++p);
  	for (;;)
  		stmt;
! 
  	/*
  	 * Parts of a for loop may be left empty.  Don't put declarations
  	 * inside blocks unless the routine is unusually complicated.
*************** main(argc, argv)
*** 187,199 ****
  		stmt;
  	} else
  		stmt;
! 		
  	/* No spaces after function names. */
  	if (error = function(a1, a2))
  		exit(error);
  
  	/*
! 	 * Unary operators don't require spaces, binary operators do. Don't
  	 * use parenthesis unless they're required for precedence, or the
  	 * statement is really confusing without them, such as:
  	 * a = b->c[0] + ~d == (e || f) || g && h ? i : j >> 1;
--- 208,220 ----
  		stmt;
  	} else
  		stmt;
! 
  	/* No spaces after function names. */
  	if (error = function(a1, a2))
  		exit(error);
  
  	/*
! 	 * Unary operators don't require spaces, binary operators do.  Don't
  	 * use parenthesis unless they're required for precedence, or the
  	 * statement is really confusing without them, such as:
  	 * a = b->c[0] + ~d == (e || f) || g && h ? i : j >> 1;
*************** main(argc, argv)
*** 204,211 ****
  	/*
  	 * Exits should be 0 on success, and 1 on failure.  Don't denote
  	 * all the possible exit points, using the integers 1 through 300.
  	 */
! 	exit(0);    /* Avoid obvious comments such as "Exit 0 on success." */
  }
  
  /*
--- 225,233 ----
  	/*
  	 * Exits should be 0 on success, and 1 on failure.  Don't denote
  	 * all the possible exit points, using the integers 1 through 300.
+ 	 * Avoid obvious comments such as "Exit 0 on success."
  	 */
! 	exit(0);
  }
  
  /*
*************** main(argc, argv)
*** 213,251 ****
   * preceeding the function.
   */
  static char *
! function(a1, a2, fl, a4)
! 	int a1, a2, a4;	/* Declare ints, too, don't default them. */
! 	float fl;	/* List in order declared, as much as possible. */
  {
  	/*
  	 * When declaring variables in functions declare them sorted by size,
! 	 * then in alphabetical order; multiple ones per line are okay.  Old
! 	 * style function declarations can go on the same line.  ANSI style
! 	 * function declarations should go in the include file "extern.h".
! 	 * If a line overflows reuse the type keyword.
  	 *
  	 * DO NOT initialize variables in the declarations.
  	 */
! 	extern u_char one;
! 	extern char two;
! 	struct foo three, *four;
! 	double five;
! 	int *six, seven, eight();
! 	char *nine, ten, eleven, twelve, thirteen, fourteen, fifteen, sixteen;
! 	char *overflow __P((void));
! 	void *mymalloc __P((u_int));
  
  	/*
  	 * 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())
  	 *
! 	 * Don't use '!' for tests unless it's a boolean, e.g. use
  	 * "if (*p == '\0')", not "if (!*p)".
   	 *
  	 * Routines returning void * should not have their return values cast
--- 235,269 ----
   * preceeding the function.
   */
  static char *
! function(int a1, int a2, float fl, int a4)
  {
  	/*
  	 * When declaring variables in functions declare them sorted by size,
! 	 * then in alphabetical order; multiple ones per line are okay.
! 	 * Function declarations (which are ANSI style) should go in the
! 	 * include file "extern.h".  If a line overflows reuse the type keyword.
  	 *
  	 * DO NOT initialize variables in the declarations.
  	 */
! 	extern u_char	 one;
! 	extern char	 two;
! 	struct foo	 three, *four;
! 	double		 five;
! 	int		*six, seven;
! 	char		*eight, *nine, ten, eleven, twelve, thirteen;
! 	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())
  	 *
! 	 * Don't use '!' for tests unless it's a boolean. E.g. use
  	 * "if (*p == '\0')", not "if (!*p)".
   	 *
  	 * Routines returning void * should not have their return values cast
*************** function(a1, a2, fl, a4)
*** 261,301 ****
  }
  
  /*
!  * Don't use ANSI function declarations unless you absolutely have to,
!  * i.e. you're declaring functions with variable numbers of arguments.
!  *
!  * ANSI function braces look like regular function braces.
   */
  void
  function(int a1, int a2)
  {
! 	...
  }
  
! /* Variable numbers of arguments should look like this. */
! #if __STDC__
  #include <stdarg.h>
- #else
- #include <varargs.h>
- #endif
  
  void
- #if __STDC__
  vaf(const char *fmt, ...)
- #else
- vaf(fmt, va_alist)
- 	char *fmt;
- 	va_dcl
- #endif
  {
! 	va_list ap;
! #if __STDC__
  	va_start(ap, fmt);
- #else
- 	va_start(ap);
- #endif
  	STUFF;
- 
  	va_end(ap);		/* No return needed for void functions. */
  }
  
--- 279,305 ----
  }
  
  /*
!  * Use ANSI function declarations.  ANSI function braces look like
!  * old-style (K&R) function braces.
   */
  void
  function(int a1, int a2)
  {
! 	/* ... */
  }
  
! /*
!  * Functions that support variable numbers of arguments should look like this.
!  */
  #include <stdarg.h>
  
  void
  vaf(const char *fmt, ...)
  {
! 	va_list	ap;
! 
  	va_start(ap, fmt);
  	STUFF;
  	va_end(ap);		/* No return needed for void functions. */
  }
  
--- cut here ---