> > From: Christophe de Dinechin <dinechin@xxxxxxxxxx> > > Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx> > --- > docs/spice_style.txt | 113 > ++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 81 insertions(+), 32 deletions(-) > > diff --git a/docs/spice_style.txt b/docs/spice_style.txt > index eb0e30ef..8e2e7363 100644 > --- a/docs/spice_style.txt > +++ b/docs/spice_style.txt > @@ -1,7 +1,7 @@ > Spice project coding style and coding conventions > ================================================= > > -Copyright (C) 2009-2016 Red Hat, Inc. > +Copyright (C) 2009-2018 Red Hat, Inc. > Licensed under a Creative Commons Attribution-Share Alike 3.0 > United States License (see > http://creativecommons.org/licenses/by-sa/3.0/us/legalcode). > > @@ -14,7 +14,16 @@ Names > > Use lower case and separate words using dashes (e.g., file-name.c, > header.h). > > -Use standard file extension for C source and header files. > +The file extensions used in the SPICE project are: > +- .c for C source > +- .cpp for C++ sources > +- .h for headers that can be included from C code > +- .hpp for headers that are strictly reserved to C++ > +- .m for Objective-C source files (currently not properly enforced) > + > +Note that .h headers may contain C++ code as long as the header can > +sucessfully be included from a C source file. > + typo "sucessfully". Why .m ? Can't wait ? :-) > > Line width > ~~~~~~~~~~ > @@ -73,7 +82,11 @@ Comments that are prefixed with `FIXME` describe a bug > that need to be fixed. Ge > ASSERT > ------ > > -Use it freely. ASSERT helps testing function arguments and function results > validity. ASSERT helps detecting bugs and improve code readability and > stability. > +Use assertions liberally. They helps testing function arguments and function > results validity. Assertions helps detecting bugs and improve code > readability and stability. > + > +Several form of assertion exist, notably: > +- spice_assert which should be preferred for any assertion related to SPICE > itself > +- glib asserts (many forms) which should be preferred for any assertion > related to the use of glib > Actually I think the original ASSERT here were supposed to be more like Windows. Note that on SPICE we never use assert as C, we always compile them in. > sizeof > ------ > @@ -93,12 +106,14 @@ Using goto is allowed in C code for error handling. In > any other case it will be > Defining Constant values > ------------------------ > > -Use defines for constant values for improving readability and ease of > changes. Alternatively, use global `const` variables. > +Use defines for constant values for improving readability and ease of > changes. > +Alternatively, use global `const` variables for individual values. > +If multiple related constants are to be defined, consider the use of > enumerations with initializers. > > Short functions > --------------- > > -Try to split code to short functions, each having simple functionality, in > order to improve code readability and re-usability. Prefix with inline short > functions or functions that were splitted for readability reason. > +Try to split code to short functions, each having simple functionality, in > order to improve code readability and re-usability. Prefix with `inline` > functions that were splitted for readability reason or that are very short. > > Return on if > ------------ Too much changes in a single patch, please split! > @@ -118,7 +133,8 @@ void function(int *n) > ... > } > ---- > -on > +over > + > [source,c] > ---- > void function(int *n) > @@ -238,15 +254,7 @@ if (condition) { > + > In case of long condition statement, prefer having additional temporary > variable over multiple line condition statement. > + > -In case of new line in condition statement. > -+ > -[source,c] > ----- > -if (long_name && very_long_name && very_long || > - var_name) { > ----- > -+ > -or indent under the round bracket using spaces > +Indent long conditionals under the opening parenthesis using spaces > + > [source,c] > ---- Why this removal? > @@ -285,6 +293,8 @@ default: > } > ---- > > +Use /* Fall through */ comments when there is no break (compilers will emit > a warning otherwise) > + > Types indentation > ~~~~~~~~~~~~~~~~~ > > @@ -330,7 +340,7 @@ Multi line macro indentation > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > [source,c] > -#define MACRO_NAME(a, b, c) { \ > +#define MACRO_NAME(a, b, c) { \ > int ab = a + c; \ > int ac = a + b; \ > int bc = b + c; \ > @@ -347,35 +357,74 @@ char *array[] = { > "item_3", > }; > > +Headers > +------- > + > +Headers should be protected against multiple inclusing using a macro that typo, "inclusing" > matches the header file name in uppercase, with all characters that are should be "upper case" > invalid in C replaced with an underscore '_': > + > +[source,h] > +--- > +#ifndef MY_MODULE_H > +#define MY_MODULE_H > + > +... > + > +#endif /* MY_MODULE_H */ > +--- > + > + > Header inclusion > ---------------- > > -Headers should be included in this order > +Headers should be included in this order: > +- config.h, which should only be included from C source files > +- [module].h, where [module].c is the corresponding implementation file > +- [module]-xyz.h, which are support headers for [module] > +- Other application headers, using #include "file.h" > +- System headers, using #include <file.h> > +- If necessary, C++ system headers, using #include <file> > + > +This order is designed to maximize chances of catching missing headers in > headers (i.e. headers that are not self-contained). > + > +In summary, Headers should be included in this order > > [source,c] > ---- > -#include <system_headers.h> > -#include <no_spice_no_system_libraries.h> > +#include "config.h" > +#include "source.h" > +#include "source-support.h" > +#include "some-other-source.h" > + > #include <spice_protocol.h> > #include <spice_common.h> > - > -#include "spice_server.h" > +#include <no_spice_no_system_libraries.h> > +#include <system_headers.h> > +#include <vector> > +#include <cstdio> > ---- > > -(note the empty line between no spice-server and spice-server headers) > +(note the empty line between application headers included with "" and system > headers included with <> > > -Also in source (no header) files you must include `config.h` at the > beginning so should start (beside comments and copyright) with > +Headers should include only the headers required to process the header > itself, and otherwise include as little as possible. > > -[source,c] > +[source,h] > ---- > -#ifdef HAVE_CONFIG_H > -#include <config.h> > -#endif > +#ifndef SOURCE_H > +#define SOURCE_H > +#include "application-header-required-for-header.h" > > -#include <system_headers.h> > -#include <no_spice_no_system_libraries.h> > -#include <spice_protocol.h> > -#include <spice_common.h> > +#include <system-header-required-for-header.h> > + > +... > > -#include "spice_server.h" > +#endif /* SOURCE_H */ I think this header changes applied everywhere should have more consents. Another reason to split this too long patch! > ---- > + > + > +Compilation > +----------- > + > +The source code should compile without warnings on all variants of GCC and > clang available. No, please! > +A patch may be rejected if it introduces new warnings. > +Warnings that appear over time due to improvements in compilers should be > fixed in dedicated patches. A patch should not mix warning fixes and other > changes. > +Any patch may adjust whitespace (e.g. eliminate trailing whitespace). > Whitespace adjustments do not require specific patches. I would agree only if the changes touch these lines. Otherwise I disagree. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel