On Mon, Nov 28, 2011 at 01:13:44PM +0000, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berra...@redhat.com> > > * HACKING: Add notes on coding style conventions > --- > HACKING | 221 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 220 insertions(+), 1 deletions(-) > > diff --git a/HACKING b/HACKING > index fe52c17..ea419e5 100644 > --- a/HACKING > +++ b/HACKING > @@ -6,4 +6,223 @@ having to run make install, there are two environment > variables > that need to be set: > > export > GI_TYPELIB_PATH=`pwd`/libvirt-glib:`pwd`/libvirt-gconfig:`pwd`/libvirt-gobject > - export > LD_LIBRARY_PATH=`pwd`/libvirt-glib/.libs:`pwd`/libvirt-gconfig/.libs:`pwd`/libvirt-gobject/.libs > \ No newline at end of file > + export > LD_LIBRARY_PATH=`pwd`/libvirt-glib/.libs:`pwd`/libvirt-gconfig/.libs:`pwd`/libvirt-gobject/.libs > + > + > +Coding style rules > + > + - Opening & closing braces for functions should be at start of line > + > + int foo(int bar) > + { > + ... > + } > + > + Not > + > + int foo(int bar) { > + ... > + } > + > + > + > + - Opening brace for if/while/for loops should be at the end of line > + > + if (foo) { > + bar; > + wizz; > + } > + > + Not > + > + if (foo) > + { > + bar; > + wizz; > + } > + > + Rationale: putting every if/while/for opening brace on a new line > + expands function length too much > + > + > + > + - If a brace needs to be used for one clause in an if/else statement, > + it should be used for both clauses, even if the other clauses are > + only single statements. eg > + > + if (foo) { > + bar; > + wizz; > + } else { > + eek; > + } > + > + Not > + > + if (foo) { > + bar; > + wizz; > + } else > + eek; > + > + > + > + - Function parameter attribute annotations should follow the parameter > + name, eg > + > + int foo(int bar G_GNUC_UNUSED) > + { > + } > + > + Not > + > + int foo(G_GNUC_UNUSED int bar) > + { > + } > + > + Rationale: Adding / removing G_GNUC_UNUSED should not cause the > + rest of the line to move around since that obscures diffs. > + > + > + > + - There should be no space between function names & open brackets eg > + > + int foo(int bar) > + { > + } > + > + Not > + > + int foo (int bar) > + { > + } > + > + > + > + - To keep lines under 80 characters (where practical), multiple parameters > + should be on new lines. Do not attempt to line up parameters vertically > + eg > + > + int foo(int bar, > + unsigned long wizz) > + { > + } > + > + Not > + > + int foo(int bar, unsigned long wizz) > + { > + } > + > + Not > + > + int foo(int bar, > + unsigned long wizz) > + { > + } > + > + Rationale: attempting vertical alignment causes bigger diffs when > + modifying code if type names change causing whitespace re-alignment. > + > + > + > + - Function declarations in headers should not attempt to line up parameters > + vertically > + > + eg > + > + int foo(int bar) > + unsigned long eek(sometype wizz) > + > + Not > + > + > + int foo(int bar) > + unsigned long eek(sometype wizz) > + > + Rationale: when making changes to headers, existing vertical alignment > + is often invalidated, requiring reindentation. This causes bigger > + diffs which obscures code review. > + > + Exception: the 6 common decls for defining a new GObject > + > + #define GVIR_TYPE_INPUT_STREAM (_gvir_input_stream_get_type > ()) > + #define GVIR_INPUT_STREAM(inst) (G_TYPE_CHECK_INSTANCE_CAST > ((inst), GVIR_TYPE_INPUT_STREAM, GVirInputStream)) > + #define GVIR_INPUT_STREAM_CLASS(class) (G_TYPE_CHECK_CLASS_CAST > ((class), GVIR_TYPE_INPUT_STREAM, GVirInputStreamClass)) > + #define GVIR_IS_INPUT_STREAM(inst) (G_TYPE_CHECK_INSTANCE_TYPE > ((inst), GVIR_TYPE_INPUT_STREAM)) > + #define GVIR_IS_INPUT_STREAM_CLASS(class) (G_TYPE_CHECK_CLASS_TYPE > ((class), GVIR_TYPE_INPUT_STREAM)) > + #define GVIR_INPUT_STREAM_GET_CLASS(inst) (G_TYPE_INSTANCE_GET_CLASS > ((inst), GVIR_TYPE_INPUT_STREAM, GVirInputStreamClass)) > + > + Rationale: These decls never change once added so vertical > + alignment of these 6 lines only, will not cause diff whitespace > + problems later. > + > + > + - Usage of goto should follow one of the following patterns, and > + label naming conventions. In particular any exit path jumps should > + obay the 'cleanup' vs 'error' label naming > + > + * Interrupted system calls: > + > + retry: > + err = func() > + if (err < 0 && errno == EINTR) > + goto retry; > + > + Alternate label name: retry_func: > + > + > + * Shared cleanup paths: > + > + int foo(int bar) > + { > + int ret = -1; > + > + > + if (something goes wrong) > + goto cleanup; > + > + ret = 0; > + cleanup: > + ...shared cleanup code... > + return ret; > + } > + > + > + * Separate error exit paths: > + > + int foo(int bar) > + { > + if (something goes wrong) > + goto error; > + > + return 0; > + > + error: > + ...error cleanup code... > + return -1; > + } > + > + > + * Separate and shared error exit paths: > + > + int foo(int bar) > + { > + int ret = -1; > + > + if (something very bad goes wrong) > + goto error; > + > + if (something goes wrong) > + goto cleanup; > + > + ret = 0; > + cleanup: > + ...shared cleanup code... > + return 0; > + > + error: > + ...error cleanup code... > + return -1; > + } > +
For what it's worth this adds a blank line to the end of the file. Christophe
pgp6YcOvp8NtU.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list