[libvirt] [PATCH libvirt-glib 1/5] Document some of the coding style conventions required

[Daniel P. Berrange]

From: "Daniel P. Berrange" <berrange at 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