Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. =================================================================== RCS file: /ftp/cvs/cvsroot/src/share/misc/style,v rcsdiff: /ftp/cvs/cvsroot/src/share/misc/style,v: warning: Unknown phrases like `commitid ...;' are present. retrieving revision 1.8 retrieving revision 1.53 diff -u -p -r1.8 -r1.53 --- src/share/misc/style 1998/09/01 06:51:09 1.8 +++ src/share/misc/style 2016/05/23 11:41:06 1.53 @@ -1,10 +1,36 @@ -/* $NetBSD: style,v 1.8 1998/09/01 06:51:09 simonb Exp $ */ +/* $NetBSD: style,v 1.53 2016/05/23 11:41:06 salazar Exp $ */ /* - * Style guide for the 4BSD KNF (Kernel Normal Form). + * The revision control tag appears first, with a blank line after it. + * Copyright text appears after the revision control tag. + */ + +/* + * The NetBSD source code style guide. + * (Previously known as 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 . Usually only one + * source file per program contains a __COPYRIGHT() section. + * Historic Berkeley code may also have an __SCCSID() section. + * Only one instance of each of these macros can occur in each file. + * Don't use newlines in the identifiers. + */ +#include +__COPYRIGHT("@(#) Copyright (c) 2008\ + The NetBSD Foundation, inc. All rights reserved."); +__RCSID("$NetBSD: style,v 1.53 2016/05/23 11:41:06 salazar Exp $"); /* * VERY important single-line comments look like this. @@ -18,13 +44,50 @@ */ /* - * Kernel include files come first; normally, you'll need - * OR , but not both! includes , - * and it's okay to depend on that. + * Attempt to wrap lines longer than 80 characters appropriately. + * Refer to the examples below for more information. + */ + +/* + * EXAMPLE HEADER FILE: + * + * A header file should protect itself against multiple inclusion. + * E.g, would contain something like: + */ +#ifndef _SYS_SOCKET_H_ +#define _SYS_SOCKET_H_ +/* + * Contents of #include file go between the #ifndef and the #endif at the end. + */ +#endif /* !_SYS_SOCKET_H_ */ +/* + * END OF EXAMPLE HEADER FILE. + */ + +/* + * If a header file requires structures, defines, typedefs, etc. from + * another header file it should include that header file and not depend + * on the including file for that header including both. If there are + * exceptions to this for specific headers it should be clearly documented + * in the headers and, if appropriate, the documentation. Nothing in this + * rule should suggest relaxation of the multiple inclusion rule and the + * application programmer should be free to include both regardless. + */ + +/* + * Kernel include files come first. */ -#include /* Non-local includes in brackets. */ +#include /* first, */ +#include /* next, */ +#include /* and then the rest, */ +#include /* sorted lexicographically. */ +#include +#include /* Non-local includes in brackets. */ -/* If it's a network program, put the network include files next. */ +/* + * If it's a network program, put the network include files next. + * Group the includes files by subdirectory. + */ #include #include #include @@ -33,9 +96,13 @@ /* * Then there's a blank line, followed by the /usr include files. - * The /usr include files should be sorted! + * The /usr include files should be sorted lexicographically! */ +#include +#include +#include #include +#include /* * Global pathnames are defined in /usr/include/paths.h. Pathnames local @@ -44,86 +111,115 @@ #include /* Then, there's a blank line, and the user include files. */ -#include "pathnames.h" /* Local includes in double quotes. */ +#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 . 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)); + * 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); +static int dirinfo(const char *, struct stat *, struct dirent *, + struct statfs *, int *, char **[]); +static void usage(void) __dead; /* declare functions that don't return dead */ /* * 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. 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;". + * 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). + */ +#define MACRO(v, w, x, y) \ +do { \ + v = (x) + (y); \ + w = (y) + 2; \ +} while (/* CONSTCOND */ 0) + +#define DOUBLE(x) ((x) * 2) + +/* Enum types are capitalized. No comma on the last element. */ +enum enumtype { + ONE, + TWO +} 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''. + * 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. + * + * 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 */ - struct mumble amumble; /* Comment for mumble */ - int bar; + struct foo *next; /* List of active foo */ + struct mumble amumble; /* Comment for mumble */ + int bar; + unsigned int baz:1, /* Bitfield; line up entries if desired */ + fuz:5, + zap:2; + uint8_t flag; }; struct foo *foohead; /* Head of global foo list */ /* Make the structure name match the typedef. */ -typedef struct _bar { - int level; +typedef struct BAR { + int level; } BAR; - + +/* C99 uintN_t is preferred over u_intN_t. */ +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. */ int -main(argc, argv) - int argc; - char *argv[]; +main(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. + * 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. */ - while ((ch = getopt(argc, argv, "abn")) != -1) + while ((ch = getopt(argc, argv, "abn:")) != -1) { switch (ch) { /* Indent the switch. */ case 'a': /* Don't indent the case. */ aflag = 1; @@ -132,28 +228,46 @@ main(argc, argv) bflag = 1; break; case 'n': + errno = 0; num = strtol(optarg, &ep, 10); - if (num <= 0 || *ep != '\0') - err("illegal number -- %s", optarg); + if (num <= 0 || *ep != '\0' || (errno == ERANGE && + (num == LONG_MAX || num == LONG_MIN)) ) + 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. + * required for control statements with only a single statement, + * unless it's a long statement. * * 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 (;;) stmt; - + + /* + * Braces are required for control statements with a single statement + * that may expand to nothing. + */ +#ifdef DEBUG_FOO +#define DPRINTF(a) printf a +#else +#define DPRINTF(a) +#endif + if (broken) { + DPRINTF(("broken is %d\n", broken)); + } + /* * Parts of a for loop may be left empty. Don't put declarations * inside blocks unless the routine is unusually complicated. @@ -165,151 +279,206 @@ main(argc, argv) /* Second level indents are four spaces. */ while (cnt < 20) - z = a + really + long + statment + that + needs + two lines + + z = a + really + long + statement + 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. + * Don't add braces that aren't necessary except in cases where + * there are ambiguity or readability issues. */ - if (test) - stmt; - else if (bar) { + if (test) { + /* + * I have a long comment here. + */ +#ifdef zorro + z = 1; +#else + b = 3; +#endif + } else if (bar) { stmt; stmt; } else stmt; - + /* No spaces after function names. */ - if (error = function(a1, a2)) - exit(error); + if ((result = function(a1, a2, a3, a4)) == NULL) + exit(1); /* - * 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. + * 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; + 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. + * 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. */ - exit(0); /* Avoid obvious comments such as "Exit 0 on success." */ + return EXIT_SUCCESS; } /* * The function type must be declared on a line by itself - * preceeding the function. + * preceding 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. */ +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. Old - * style function declarations can go on the same line. ANSI style - * function declarations should go in the include file "extern.h". + * 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. * - * DO NOT initialize variables in the declarations. + * Avoid initializing variables in the declarations; move + * declarations next to their first use, and initialize + * opportunistically. This avoids over-initialization and + * accidental bugs caused by declaration reordering. */ 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)); + 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: + * against NULL. I.e. use: * - * (p = f()) == NULL + * (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. + * The notable exception here is variadic functions. Since our + * code is designed to compile and work on different environments + * where we don't have control over the NULL definition (on NetBSD + * it is defined as ((void *)0), but on other systems it can be + * defined as (0) and both definitions are valid under ANSI C), it + * it advised to cast NULL to a pointer on variadic functions, + * because on machines where sizeof(pointer) != sizeof(int) and in + * the absence of a prototype in scope, passing an un-casted NULL, + * will result in passing an int on the stack instead of a pointer. + * + * 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 more specific pointer types. + * + * Prefer sizeof(*var) over sizeof(type) because if type changes, + * the change needs to be done in one place. * * Use err/warn(3), don't roll your own! */ - if ((four = malloc(sizeof(struct foo))) == NULL) + if ((four = malloc(sizeof(*four))) == NULL) err(1, NULL); if ((six = (int *)overflow()) == NULL) errx(1, "Number overflowed."); - return (eight); + + /* No parentheses are needed around the return value. */ + return eight; } /* - * 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. - */ -function(int a1, int a2) -{ - ... + * 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[]) +{ /* Insert an empty line if the function has no local variables. */ + + /* + * In system libraries, catch obviously invalid function arguments + * using _DIAGASSERT(3). + */ + _DIAGASSERT(p != NULL); + _DIAGASSERT(filedesc != -1); + + if (stat(p, sb) < 0) + err(1, "Unable to stat %s", p); + + /* + * To printf quantities that might be larger than "long", include + * , cast quantities to intmax_t or uintmax_t and use + * PRI?MAX constants. + */ + (void)printf("The size of %s is %" PRIdMAX " (%#" PRIxMAX ")\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); + + /* + * There are similar constants that should be used with the *scanf(3) + * family of functions: SCN?MAX, SCN?64, etc. + */ } -/* Variable numbers of arguments should look like this. */ -#if __STDC__ +/* + * Functions that support variable numbers of arguments should look like this. + * (With the #include appearing at the top of the file with the + * other include files.) + */ #include -#else -#include -#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. */ + va_end(ap); + /* No return needed for void functions. */ } static void -usage() -{ /* Insert an empty line if the function has no local variables. */ +usage(void) +{ /* * 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. * - * 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. + * Use getprogname() instead of hardcoding the program name. * - * "usage: f [-ade] [-b b_arg] [-m m_arg] req1 req2 [opt1 [opt2]]\n" + * "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: f [-ab]\n"); - exit(1); + (void)fprintf(stderr, "usage: %s [-ab]\n", getprogname()); + exit(EXIT_FAILURE); }