[v4,09/12] Fix the style guide for headers

Submitted by Christophe de Dinechin on Feb. 15, 2018, 4:06 p.m.

Details

Message ID 20180215160625.75406-10-christophe@dinechin.org
State New
Headers show
Series "Updates to the style guide" ( rev: 3 ) in Spice

Not browsing as part of any series.

Commit Message

Christophe de Dinechin Feb. 15, 2018, 4:06 p.m.
From: Christophe de Dinechin <dinechin@redhat.com>

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.

Changes since v3:
- Add comment indicating that this is a reversal from an earlier guideline,
  and that the code base is not adjusted yet.

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
 docs/spice_style.txt | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

Patch hide | download patch | download mbox

diff --git a/docs/spice_style.txt b/docs/spice_style.txt
index 90ad577d..c28d7be3 100644
--- a/docs/spice_style.txt
+++ b/docs/spice_style.txt
@@ -442,34 +442,50 @@  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).
+
+NOTE: Historically, a different order was recommended by this style guide. As a result, most of the code base presently puts system headers first, and some headers are not self-contained. Patches to adjust to the new style are welcome.
+
+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 */
 ----
 
 C++