Re: [PATCH v3 11/11] Rewrite the style guide for headers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This one sounds more like an RFC to me, as from a quick look in server/,
this is not the style currently in use.

Christophe

On Thu, Feb 08, 2018 at 12:25:31PM +0100, Christophe de Dinechin wrote:
> From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
> 
> As written, the headers style guide looks quite wrong. In particular,
> it places headers in an order that makes it hard to detect hidden
> dependencies in SPICE headers.
> 
> These rules can be enforced by the .clang-format proposed in earlier patch,
> locally if you use the Emacs clang-format.el integration supplied with LLVM.
> 
> Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
> ---
>  docs/spice_style.txt | 44 +++++++++++++++++++++++++++++---------------
>  1 file changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/docs/spice_style.txt b/docs/spice_style.txt
> index 108a57a5..ff505b2a 100644
> --- a/docs/spice_style.txt
> +++ b/docs/spice_style.txt
> @@ -407,34 +407,48 @@ Historically, some headers added underscores liberally, e.g. MY_MODULE_H_. This
>  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]
>  ----
> -#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 */
>  ----
>  
>  
> -- 
> 2.13.5 (Apple Git-94)
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]