7:13 a.m.
From: "Daniel P. Berrange" <berrange(a)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;
+ }
+
--
1.7.6.4