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

[Christophe Fergeau]

On Mon, Nov 28, 2011 at 01:13:44PM +0000, Daniel P. Berrange wrote:
> 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;
> +       }
> +

For what it's worth this adds a blank line to the end of the file.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111128/32696ce3/attachment-0001.sig>