From e1747e09e7cc6771dca1a1702e42e6401dbeebed Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Thu, 27 Apr 2000 02:00:44 +0000 Subject: [PATCH] Vertical whitespace is encouraged for improved code legibility by grouping closely related statements and then separating them with a single empty line. Lines should not be longer than 79 characters, even if it requires violating the indentation rules to do so. Since ANSI is assumed, the best way to deal with strings that extend past column 79 is to break them into two or more sections separated from each other by a newline and indentation. (w/example) Note that should be included by any public header file to get the ISC_LANG_BEGINDECLS and ISC_LANG_ENDDECLS macros used so the correct name-mangling happens for function declarations when C++ programs include the file. should be included for private header files or for public files that do not declare any functions. (w/example) Fixed < and > use in sample header file. The config.h file must never be included by any public header file. The comma operator should not be used to form compound statements. (w/example) Generally speaking, when a control statement (if, for or while) has only a single action associated with it, then no bracing is used around the statement. Exceptions include when the compiler would complain about an ambiguous else clause, or when extra bracing improves the readability (a judgement call biased toward not having the braces). Do not put a space after the "sizeof" operator name, and also parenthesize its argument, as in malloc(4 * sizeof(long)). Do not put a space after a cast. (w/example)

The Ternary Operator

(w/example) The ?: operator should mostly be avoided. It is tolerated when deciding what value to pass as a parameter to a function, such as frequently happens with printf, and also when a simple (non-compound) value is being used in assignment or as part of a calculation. In particular, using the ternary operator to specify a return value is verboten. (Well, Bob didn't tell me *forbidden* when he first said this to me long ago, but I got the impression he really did not like it.) Variables should not have their values assigned or changed when being passed as parameters, except perhaps for the increment and decrement operators. (This came up when I found something much like this in one of our files: malloc(size = 20); All public interfaces to functions, macros, typedefs, and variables provided by the library, should use names of the form {library}_{module}_{what}, such as: isc_buffer_t /* typedef */ dns_name_setbuffer(name, buffer) /* function */ ISC_LIST_HEAD(list) /* macro */ isc_commandline_argument /* variable */ however, structures which are typedef'd generally have the name of the typedef sans the final _t: struct dns_rbtnode { /* ... members ... */ } Generally speaking macros are defined with all capital letters, but this is not universally consistent (eg, numerous isc_buffer_{foo} macros). The {module} and {what} segments of the name do not have underscores separating natural word elements, as demonstrated in isc_commandline_argument and dns_name_setbuffer above. The {module} part is usually the same as the basename of the source file, but sometimes other {module} interfaces appear within one file, such as dns_label_* interfaces in lib/dns/name.c. However, in the public libraries the file name must be the same as some module interface provided by the file; e.g., dns_rbt_* interfaces would not be declared in a file named redblack.c (in lieu of any other dns_redblack_* interfaces in the file). The one notable exception to this naming rule is the interfaces provided by . There's a large caveat associated with the public description of this file that it is hazardous to use because it pollutes the general namespace.

Shared Private Interfaces

When a module provides an interface for internal use by other modules in the library, it should use the same naming convention described for the public interfaces, except {library} and {module} are separated by a double-underscore. This indicates that the name is internal, its API is not as formal as the public API, and thus it might change without any sort of notice. --- CHANGES | 3 + doc/dev/coding.html | 174 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 155 insertions(+), 22 deletions(-) diff --git a/CHANGES b/CHANGES index 6b705072ab..d74d2914c0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ + 105. [doc] doc/dev/coding.html expanded with other + implicit conventions the developers have used. + 104. [bug] Made compress_add and compress_find static to lib/dns/compress.c. diff --git a/doc/dev/coding.html b/doc/dev/coding.html index 5af59933ba..2e1c6c7746 100644 --- a/doc/dev/coding.html +++ b/doc/dev/coding.html @@ -25,8 +25,25 @@ indicated with "_": _______"a", "printf"); -

Line Length

Lines should not be longer than 80 characters, -even if it requires violating the indentation rules to do so. +

Vertical Whitespace

+Vertical whitespace is also encouraged for improved code legibility by +grouping closely related statements and then separating them with a +single empty line. There should not, however, be more than one empty +adjacent line anywhere. + +

Line Length

+Lines should not be longer than 79 characters, even if it requires +violating the indentation rules to do so. Since ANSI is assumed, the +best way to deal with strings that extend past column 79 is to break +them into two or more sections separated from each other by a newline +and indentation: + +

+        			  puts("This string got very far to the "
+                                       "left and wrapped.  ANSI catenation "
+                                       "rules will turn this into one
+                                       "long string.");
+

Comments

Comments should be used anytime they improve the readability of the code.

@@ -63,7 +80,13 @@ The following lint and lint-like comments should be used where appropriate: .h files should not rely on other files having been included. .h files should prevent multiple inclusion. The OS is assumed to prevent multiple inclusion of its .h files.

-.h files that define modules should have a structure like the following:

+.h files that define modules should have a structure like the +following. Note that should be included by any public +header file to get the ISC_LANG_BEGINDECLS and ISC_LANG_ENDDECLS +macros used so the correct name-mangling happens for function +declarations when C++ programs include the file. should +be included for private header files or for public files that do not +declare any functions.


 /*
  * Copyright (C) 1998  Internet Software Consortium.
@@ -90,44 +113,48 @@ multiple inclusion of its .h files.

  *****/
 
 /*
- * 
+ * (Module name here.)
  *
- * 
+ * (One line description here.)
  *
- * 
+ * (Extended description and notes here.)
  *
  * MP:
- *	
+ *	(Information about multiprocessing considerations here, e.g. locking
+ *	 requirements.)
  *
  * Reliability:
- *	
+ *	(Any reliability concerns should be mentioned here.)
  *
  * Resources:
- *	
+ *	(A rough guide to how resources are used by this module.)
  *
  * Security:
- *	
+ *	(Any security issues are discussed here.)
  *
  * Standards:
- *	
+ *	(Any standards relevant to the module are listed here.)
  */
 
 /***
  *** Imports
  ***/
 
-/* <#includes here> */
+/* #includes here. */
+#include <isc/lang.h>
 
 /***
  *** Types
  ***/
 
-/*  */
+/* (Type definitions here.) */
 
 /***
  *** Functions
  ***/
+ISC_LANG_BEGINDECLS
+/* (Function declarations here, with full prototypes.) */
+ISC_LANG_ENDDECLS
 
 #endif /* ISC_WHATEVER_H */
 
@@ -135,18 +162,21 @@ multiple inclusion of its .h files.

 
 
C Source

 
Including Interfaces (.h files)

-The first file to be included must be config.h.
-Try to include only necessary files, not everything under the
-sun.

+The first file to be included in a C source file must be config.h.
+The config.h file must never be included by any public header file
+(that is, any header file that will be installed by "make install").
+Try to include only necessary files, not everything under the sun.

 Operating-system-specific files should not be included by most modules.

 Include UNIX "sys" .h files before ordinary C includes.

 
 
Statements

-There should be at most one statement per line.

+There should be at most one statement per line.  The comma operator
+should not be used to form compound statements.

 Bad:

 

 	if (i > 0) {
 		printf("yes\n"); i = 0; j = 0;
+                x = 4, y *= 2;
 	}
 

 
Functions

@@ -170,11 +200,20 @@ g(int i, /* other args here */
 }
-

Curly Braces

Curly Braces do not get their own indentation. +

Curly Braces

+Curly Braces do not get their own indentation. An opening brace does not start a new line. The statements enclosed by the braces should not be on the same line as the opening or closing brace. A closing brace should be the only thing on the line, unless it's part of an else clause.

+ +Generally speaking, when a control statement (if, for or +while) has only a single action associated with it, then no +bracing is used around the statement. Exceptions include when the +compiler would complain about an ambiguous else clause, or when extra +bracing improves the readability (a judgement call biased toward not +having the braces).

+ Good:


 static void
@@ -200,13 +239,15 @@ void f(int i)
 
 
Spaces

 
    
-
  • Do put a space between operators like '+', '==', etc.
+
  • Do put a space between operators like '=', '+', '==', etc.
 
  • Do put a space after ','.
 
  • Do put a space after ';' in a 'for' statement.
 
  • Do put a space after 'return', and also parenthesize the return value.
 

 
    
 
  • Do not put a space between a variable or function name and '(' or '['.
+
  • Do not put a space after the "sizeof" operator name, and also
+parenthesize its argument, as in malloc(4 * sizeof(long)).
 
  • Do not put a space immediately after a '(' or immediately before a ')',
 unless it improves readability.  The same goes for '[' and ']'.
 
  • Do not put a space before '++' or '--' when used in
@@ -219,11 +260,12 @@ either side of '->'.
 
  • Do not put a space after '~'.
 
  • The '|' operator may either have a space on both sides or it may have no
 spaces.
+
  • Do not put a space after a cast.
 

 
 
Return Values

 If a function returns a value, it should be cast to (void) if you don't
-care what the value is.  (Exception for printf()?)

+care what the value is, except for printf

 
 All error conditions must be handled.

 
@@ -266,7 +308,13 @@ comparisons between signed and unsigned integers should be avoided;
 suppressing the warnings with casts is not desireable.

 
 
Casting

-Casting should be avoided when possible.

+Casting should be avoided when possible.  When it is necessary, there
+should be no space between the cast and what is being cast.

+
+Bad (obviously for more than one reason ...):
+

+	(void) malloc(SMBUF);
+

 
 
Clear Success or Failure

 A function should report success or failure, and do so accurately.  It
@@ -354,6 +402,88 @@ Bad:
 	}
+

The Ternary Operator

+The ?: operator should mostly be avoided. It is tolerated when +deciding what value to pass as a parameter to a function, such as +frequently happens with printf, and also when a simple (non-compound) +value is being used in assignment or as part of a calculation. +In particular, using the ternary operator to specify a return value is +verboten.

+ +Good: +


+	printf("%c is%s a number.\n", c, isdigit(c) ? "" " NOT");
+        l = (l1 < l2) ? l1 : l2;
+        if (gp.length + (go < 16384 ? 2 : 3) >= name->length) {
+           ...
+        }
+
+ +Bad: +

+	return (success ? ISC_R_SUCESS : ISC_R_FAILURE);
+
+ +

Assignment in Parameters

+Variables should not have their values assigned or changed when being +passed as parameters, except perhaps for the increment and decrement +operators.

+ +Bad: +


+	malloc(size = 20);
+
+ +Ok: +

+	fputc(c++, stdout);
+
+ +

Namespace

+

Public Interfaces

+All public interfaces to functions, macros, typedefs, and +variables provided by the library, should use names of the form +{library}_{module}_{what}, such as: +

+	isc_buffer_t				/* typedef */
+        dns_name_setbuffer(name, buffer)	/* function */
+        ISC_LIST_HEAD(list)			/* macro */
+        isc_commandline_argument		/* variable */
+
+however, structures which are typedef'd generally have the name of the +typedef sans the final _t: +

+	struct dns_rbtnode {
+        	/* ... members ... */
+	}
+
+Generally speaking macros are defined with all capital letters, but +this is not universally consistent (eg, numerous isc_buffer_{foo} +macros).

+The {module} and {what} segments of the name do not have underscores +separating natural word elements, as demonstrated in +isc_commandline_argument and dns_name_setbuffer above. The {module} +part is usually the same as the basename of the source file, but +sometimes other {module} interfaces appear within one file, such as +dns_label_* interfaces in lib/dns/name.c. However, in the public +libraries the file name must be the same as some module interface +provided by the file; e.g., dns_rbt_* interfaces would not be declared +in a file named redblack.c (in lieu of any other dns_redblack_* +interfaces in the file).

+ +The one notable exception to this naming rule is the interfaces +provided by . There's a large caveat associated with the +public description of this file that it is hazardous to use because it +pollutes the general namespace.

+ +

Shared Private Interfaces

+When a module provides an interface for internal use by other modules +in the library, it should use the same naming convention +described for the public interfaces, except {library} and {module} +are separated by a double-underscore. This indicates that the name is +internal, its API is not as formal as the public API, and thus it +might change without any sort of notice. +

Initialization

When an object is allocated from the heap, all fields in the object must be initialized.