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

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

 



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.

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@xxxxxxxxxx>
---
 docs/spice_style.txt | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

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++
-- 
2.13.5 (Apple Git-94)

_______________________________________________
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]