Xen devel - Mar 2012 - [PATCH] Document the current coding style conventions.

From: David Vrabel <david.vrabel@citrix.com>

Based on a version originally posted in 2007.
http://lists.xen.org/archives/html/xen-devel/2007-06/msg00070.html

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 docs/misc/coding_style.txt |  100 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 100 insertions(+), 0 deletions(-)
 create mode 100644 docs/misc/coding_style.txt

diff --git a/docs/misc/coding_style.txt b/docs/misc/coding_style.txt
new file mode 100644
index 0000000..2ece1ec
--- /dev/null
+++ b/docs/misc/coding_style.txt
@@ -0,0 +1,100 @@
+Coding Style for the Xen Hypervisor
+==================================+
+This coding style is used for C code that is part of the Xen
+Hypervisor itself, or part of the associated user space tools.  Guest
+operating system code may use the native coding style of that code
+base.  The goal of this file is to document the main points of the Xen
+coding style, particularly where they differ from normal Linux
+convention.
+
+libxl has its own coding style documented in tools/libxl/CODING_STYLE.
+
+Indentation
+-----------
+
+Indenting uses spaces, not tabs - in contrast to Linux.  An indent
+level consists of four spaces.  Code within blocks is indented by one
+extra indent level.  The enclosing braces of a block are indented the
+same as the code _outside_ the block.  e.g.
+
+void fun(void)
+{
+    /* One level of indent. */
+
+    {
+        /* A second level of indent. */
+    }
+}
+
+White space
+-----------
+
+Space characters are used to spread out logical statements, such as in
+the condition of an if or while.  Spaces are placed between the
+keyword and the brackets surrounding the condition, between the
+brackets and the condition itself, and around binary operators (except
+the structure memory operators, ''.'' and
''->''). e.g.
+
+if ( (wibble & wombat) == 42 )
+{
+    ...
+
+There should be no trailing white space at the end of lines (including
+after the opening /* of a comment block).
+
+Bracing
+-------
+
+Braces (''{'' and ''}'') are usually placed on
a line of their own, except
+for the do/while loop.  This is unlike the Linux coding style and
+unlike K&R.  do/while loops are an exception. e.g.:
+
+if ( condition )
+{
+    /* Do stuff. */
+}
+
+while ( condition )
+{
+    /* Do stuff. */
+}
+
+do {
+    /* Do stuff. */
+} while ( condition );
+
+etc.
+
+Braces should be omitted for blocks with a single statement. e.g.,
+
+if ( condition )
+    single_statement();
+
+Comments
+--------
+
+Only C style /* ... */ comments are to be used.  C++ style // comments
+should not be used in submitted code.  Multi-word comments should
+begin with a capital letter and end with a full stop.
+
+/*
+ * Example, multi-line comment block.
+ *
+ * Note beginning and end markers on separate lines and leading
''*''.
+ */
+
+Emacs local variables
+---------------------
+
+A comment block containing local variables for emacs is permitted at
+the end of files.  It should be:
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.7.2.5

>>> On 14.03.12 at 18:27, David Vrabel <david.vrabel@citrix.com>
wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> Based on a version originally posted in 2007.
> http://lists.xen.org/archives/html/xen-devel/2007-06/msg00070.html 
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  docs/misc/coding_style.txt |  100 
> ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 100 insertions(+), 0 deletions(-)
>  create mode 100644 docs/misc/coding_style.txt
> 
> diff --git a/docs/misc/coding_style.txt b/docs/misc/coding_style.txt
> new file mode 100644
> index 0000000..2ece1ec
> --- /dev/null
> +++ b/docs/misc/coding_style.txt
> @@ -0,0 +1,100 @@
> +Coding Style for the Xen Hypervisor
> +==================================> +
> +This coding style is used for C code that is part of the Xen
> +Hypervisor itself, or part of the associated user space tools.  Guest
> +operating system code may use the native coding style of that code
> +base.  The goal of this file is to document the main points of the Xen
> +coding style, particularly where they differ from normal Linux
> +convention.
> +
> +libxl has its own coding style documented in tools/libxl/CODING_STYLE.
> +
> +Indentation
> +-----------
> +
> +Indenting uses spaces, not tabs - in contrast to Linux.  An indent
> +level consists of four spaces.  Code within blocks is indented by one
> +extra indent level.  The enclosing braces of a block are indented the
> +same as the code _outside_ the block.  e.g.
> +
> +void fun(void)
> +{
> +    /* One level of indent. */
> +
> +    {
> +        /* A second level of indent. */
> +    }
> +}
An exception should probably be declared for code pulled over from
Linux almost unmodified.

Further, Linux''es document doesn''t say anything about the
indentation of long function parameter and argument lists. Do we
want to leave this to the authors'' taste as well?
> +
> +White space
> +-----------
> +
> +Space characters are used to spread out logical statements, such as in
> +the condition of an if or while.  Spaces are placed between the
> +keyword and the brackets surrounding the condition, between the
> +brackets and the condition itself, and around binary operators (except
> +the structure memory operators, ''.'' and
''->''). e.g.
> +
> +if ( (wibble & wombat) == 42 )
> +{
> +    ...
> +
> +There should be no trailing white space at the end of lines (including
> +after the opening /* of a comment block).
> +
> +Bracing
> +-------
> +
> +Braces (''{'' and ''}'') are usually
placed on a line of their own, except
> +for the do/while loop.  This is unlike the Linux coding style and
> +unlike K&R.  do/while loops are an exception. e.g.:
> +
> +if ( condition )
> +{
> +    /* Do stuff. */
> +}
> +
> +while ( condition )
> +{
> +    /* Do stuff. */
> +}
> +
> +do {
> +    /* Do stuff. */
> +} while ( condition );
> +
> +etc.
> +
> +Braces should be omitted for blocks with a single statement. e.g.,
"may" is probably the better term, as there''s quite a number
of
examples where this is not the case. (I would personally like the
more strict form you gave, as I dislike those superfluous braces).
> +
> +if ( condition )
> +    single_statement();
> +
> +Comments
> +--------
> +
> +Only C style /* ... */ comments are to be used.  C++ style // comments
> +should not be used in submitted code.  Multi-word comments should
> +begin with a capital letter and end with a full stop.
> +
> +/*
> + * Example, multi-line comment block.
> + *
> + * Note beginning and end markers on separate lines and leading
''*''.
> + */
The statement prior to the example only mentions multi-word
comments, whereas the example is a multi-line one. Perhaps the
statement could be extended to demand the /* and */ to be on
separate lines for multi-line comments.

Nothing is being said on maximum line length. In particular it''d be
nice
to clarify whether we want to follow Linux''es relatively recent (in
3.1)
move towards not breaking up printk() messages (which I''m in favor
of, and would even volunteer to clean up, as I really dislike having to
guess how long lines got broken up in the sources - it wasn''t just
once that I had to make a couple of tries until I managed to locate a
certain message).

Jan
> +
> +Emacs local variables
> +---------------------
> +
> +A comment block containing local variables for emacs is permitted at
> +the end of files.  It should be:
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel

On Wed, 2012-03-14 at 17:27 +0000, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> Based on a version originally posted in 2007.
> http://lists.xen.org/archives/html/xen-devel/2007-06/msg00070.html
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  docs/misc/coding_style.txt |  100
++++++++++++++++++++++++++++++++++++++++++++
Thanks for this!

I think this should be CODING_STYLE at the top level (in the vein of
top-level COPYRIGHT + local overrides) and commence with an intro such
as:

        The Xen coding style described below is the coding style used by
        the Xen hypervisor itself (xen/*) as well as various associated
        low-level libraries (e.g. tools/libxc/*).
        
        An exception is made for files which are imported from an
        external source. In these cases the prevailing coding style of
        the upstream source is generally used (commonly the Linux coding
        style).
        
        Other parts of the code base may use other coding styles,
        sometimes explicitly (e.g. tools/libxl/CODING_STYLE) but often
        implicitly (Linux coding style is fairly common). In general you
        should copy the style of the surrounding code. If you are unsure
        please ask.

[...]
> +Emacs local variables
> +---------------------
> +
> +A comment block containing local variables for emacs is permitted at
> +the end of files.  It should be:
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
You mentioned a while ago that the comment block we''ve been cutting and
pasting around the place was buggy. Is this the prevailing buggy one or
did you fix it up here?

Ian.

On 15/03/12 09:34, Ian Campbell wrote:
> On Wed, 2012-03-14 at 17:27 +0000, David Vrabel wrote:
>> +Emacs local variables
>> +---------------------
>> +
>> +A comment block containing local variables for emacs is permitted at
>> +the end of files.  It should be:
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
> 
> You mentioned a while ago that the comment block we''ve been
cutting and
> pasting around the place was buggy. Is this the prevailing buggy one or
> did you fix it up here?
This is the correct one (according to the documentation).  Should all
the existing ones be fixed up all at once?

David

On Thu, 2012-03-15 at 10:54 +0000, David Vrabel wrote:
> On 15/03/12 09:34, Ian Campbell wrote:
> > On Wed, 2012-03-14 at 17:27 +0000, David Vrabel wrote:
> >> +Emacs local variables
> >> +---------------------
> >> +
> >> +A comment block containing local variables for emacs is permitted
at
> >> +the end of files.  It should be:
> >> +
> >> +/*
> >> + * Local variables:
> >> + * mode: C
> >> + * c-file-style: "BSD"
> >> + * c-basic-offset: 4
> >> + * indent-tabs-mode: nil
> >> + * End:
> >> + */
> > 
> > You mentioned a while ago that the comment block we''ve been
cutting and
> > pasting around the place was buggy. Is this the prevailing buggy one
or
> > did you fix it up here?
> 
> This is the correct one (according to the documentation).
and the docs are right i.e. it does the right thing? ;-)
> Should all the existing ones be fixed up all at once?
I guess it is just a sed invocation? In which case it seems reasonable
to do it as a big bang change.

Ian.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, Mar 14, 2012 at 6:27 PM, David Vrabel <david.vrabel@citrix.com>
wrote:
>
> +Bracing
> +-------
> +
> +Braces (''{'' and ''}'') are usually
placed on a line of their own, except
> +for the do/while loop.  This is unlike the Linux coding style and
> +unlike K&R.  do/while loops are an exception. e.g.:
> +
> +if ( condition )
> +{
> +    /* Do stuff. */
> +}
> +
>
Would it be worth to cite what''s the preferred style in case of
else/else if ? I guess something like the below?

if ( xxx )
{
    zzz
} else {
    yyy
}
> +Braces should be omitted for blocks with a single statement. e.g.,
> +
> +if ( condition )
> +    single_statement();
> +
>
And what if the *then* branch has one statement and the *else*
has more (or vice versa)?

Or is that too much of a nitpicking? :-)

Thanks and Regards,
Dario

- --
<<This happens because I choose it to happen!>> (Raistlin Majere)
-
---------------------------------------------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)

iEYEARECAAYFAk9iM28ACgkQk4XaBE3IOsSYgACeIFG4qzHBPHIzqPb7PdPkZZmm
01YAoIakO0RrLJ0eVyB4NXXuoMDKIxAf
=4gCT
-----END PGP SIGNATURE-----

>>> On 15.03.12 at 19:23, Dario Faggioli <raistlin@linux.it>
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On Wed, Mar 14, 2012 at 6:27 PM, David Vrabel
<david.vrabel@citrix.com> wrote:
>>
>> +Bracing
>> +-------
>> +
>> +Braces (''{'' and ''}'') are usually
placed on a line of their own, except
>> +for the do/while loop.  This is unlike the Linux coding style and
>> +unlike K&R.  do/while loops are an exception. e.g.:
>> +
>> +if ( condition )
>> +{
>> +    /* Do stuff. */
>> +}
>> +
>>
> Would it be worth to cite what''s the preferred style in case of
> else/else if ? I guess something like the below?
> 
> if ( xxx )
> {
>     zzz
> } else {
>     yyy
> }

}
else
{
>> +Braces should be omitted for blocks with a single statement. e.g.,
>> +
>> +if ( condition )
>> +    single_statement();
>> +
>>
> And what if the *then* branch has one statement and the *else*
> has more (or vice versa)?
if ( ... )
    <statement>
else
{
    <statements>
}

or, respectively,

if ( ... )
{
    <statements>
}
else
     <statement>

would be my view on this.
> Or is that too much of a nitpicking? :-)
I''d say this should be taken as implicit from the other pieces.

Jan
> Thanks and Regards,
> Dario
> 
> - --
> <<This happens because I choose it to happen!>> (Raistlin
Majere)
> -
---------------------------------------------------------------------------------------------------
> Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli 
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.12 (GNU/Linux)
> 
> iEYEARECAAYFAk9iM28ACgkQk4XaBE3IOsSYgACeIFG4qzHBPHIzqPb7PdPkZZmm
> 01YAoIakO0RrLJ0eVyB4NXXuoMDKIxAf
> =4gCT
> -----END PGP SIGNATURE-----
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel

On 15/03/12 09:34, Ian Campbell wrote:
> On Wed, 2012-03-14 at 17:27 +0000, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> Based on a version originally posted in 2007.
>> http://lists.xen.org/archives/html/xen-devel/2007-06/msg00070.html
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> ---
>>  docs/misc/coding_style.txt |  100
++++++++++++++++++++++++++++++++++++++++++++
> 
> Thanks for this!
> 
> I think this should be CODING_STYLE at the top level (in the vein of
> top-level COPYRIGHT + local overrides).
The way I think most people will discover this will be via the
submitting patch page on the wiki.  If this doc is in the top-level can
it be linked from the wiki?
> [snip useful intro text]
I''ve incorporated this, thanks.

David

On Mon, 2012-03-19 at 11:18 +0000, David Vrabel wrote:
> On 15/03/12 09:34, Ian Campbell wrote:
> > On Wed, 2012-03-14 at 17:27 +0000, David Vrabel wrote:
> >> From: David Vrabel <david.vrabel@citrix.com>
> >>
> >> Based on a version originally posted in 2007.
> >> http://lists.xen.org/archives/html/xen-devel/2007-06/msg00070.html
> >>
> >> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> >> ---
> >>  docs/misc/coding_style.txt |  100
++++++++++++++++++++++++++++++++++++++++++++
> > 
> > Thanks for this!
> > 
> > I think this should be CODING_STYLE at the top level (in the vein of
> > top-level COPYRIGHT + local overrides).
> 
> The way I think most people will discover this will be via the
> submitting patch page on the wiki.  If this doc is in the top-level can
> it be linked from the wiki?
I think so, using README as an example:
http://xenbits.xen.org/hg/xen-unstable.hg/file/tip/README
or
http://xenbits.xen.org/hg/xen-unstable.hg/raw-file/tip/README

It doesn''t actually need to be top level for this to work (e.g.
http://xenbits.xen.org/hg/xen-unstable.hg/raw-file/tip/tools/libxl/CODING_STYLE
), but I think top level is the right place for the new document.

I''d also have no objection to a patch to docs/Makefile such that these
files were propagated into xenbits/xen.org/docs/xen-unstable etc.

Ian.
> 
> > [snip useful intro text]
> 
> I''ve incorporated this, thanks.
> 
> David