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

Re: [libvirt] [PATCH] Fix _FILE_OFFSET_BITS re-definition



john levon sun com wrote:
> # HG changeset patch
> # User john levon sun com
> # Date 1229399267 28800
> # Node ID db36391b739c117f5887388f65f31e6a9d2d361b
> # Parent  020f8b8e9340287a6ab3d869b359e39b905cd0ff
> Fix _FILE_OFFSET_BITS re-definition
>
> Since config.h contains the _FILE_OFFSET_BITS setting (a little dubious
> in itself), it must be the first header included, otherwise system
> headers can define _FILE_OFFSET_BITS differently themselves.
>
> Signed-off-by: John Levon <john levon sun com>
>
> diff --git a/src/node_device_hal.c b/src/node_device_hal.c
> --- a/src/node_device_hal.c
> +++ b/src/node_device_hal.c
> @@ -21,10 +21,11 @@
>   * Author: David F. Lively <dlively virtualiron com>
>   */
>
> +#include <config.h>
> +
>  #include <stdio.h>
>  #include <stdlib.h>
>
> -#include <config.h>
>  #include <libhal.h>
>
>  #include "node_device_conf.h"

ACK

FYI, there are 3 other cases where the "include <config.h> first"
rule is broken:

  docs/examples/info1.c
  docs/examples/suspend.c
  qemud/remote_protocol.c

so I've just written a new syntax-check rule to enforce this.
It fixes only the last one.  Since the two examples are not
necessarily built using libvirt's own build system, so they
are exempted.

So with your patch above and the following one,
the new "make sc_require_config_h_first" part of "make syntax-check"
passes:

>From 15afb090ac28ab6b9274fb827eb1c1c939db4104 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering redhat com>
Date: Wed, 17 Dec 2008 10:49:04 +0100
Subject: [PATCH] enforce the "include <config.h> first" rule

* qemud/Makefile.am: Ensure that the generated remote_protocol.c
includes <config.h> first.
* qemud/remote_protocol.c: Regenerate.
* Makefile.maint (sc_require_config_h_first): New rule, so that
"make syntax-check" enforces this.
* .x-sc_require_config_h_first: New file.
* Makefile.am (.x-sc_require_config_h_first): Add it.
---
 .x-sc_require_config_h_first |    2 ++
 ChangeLog                    |   11 +++++++++++
 Makefile.am                  |    1 +
 Makefile.maint               |   15 +++++++++++++++
 qemud/Makefile.am            |    9 +++++++--
 qemud/remote_protocol.c      |    2 +-
 6 files changed, 37 insertions(+), 3 deletions(-)
 create mode 100644 .x-sc_require_config_h_first

diff --git a/.x-sc_require_config_h_first b/.x-sc_require_config_h_first
new file mode 100644
index 0000000..58a8878
--- /dev/null
+++ b/.x-sc_require_config_h_first
@@ -0,0 +1,2 @@
+^docs/examples/info1\.c$
+^docs/examples/suspend\.c$
diff --git a/ChangeLog b/ChangeLog
index 08a1cb5..36c1278 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2008-12-17  Jim Meyering  <meyering redhat com>
+
+	enforce the "include <config.h> first" rule
+	* qemud/Makefile.am: Ensure that the generated remote_protocol.c
+	includes <config.h> first.
+	* qemud/remote_protocol.c: Regenerate.
+	* Makefile.maint (sc_require_config_h_first): New rule, so that
+	"make syntax-check" enforces this.
+	* .x-sc_require_config_h_first: New file.
+	* Makefile.am (.x-sc_require_config_h_first): Add it.
+
 Wed Dec 17 08:02:01 +0100 2008 Jim Meyering <meyering redhat com>

 	fix numa-related (and kernel-dependent) test failures
diff --git a/Makefile.am b/Makefile.am
index d40a151..758ad50 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,6 +14,7 @@ EXTRA_DIST = \
   libvirt.pc libvirt.pc.in \
   $(man_MANS) autobuild.sh \
   .x-sc_avoid_if_before_free \
+  .x-sc_require_config_h_first \
   .x-sc_prohibit_strcmp \
   .x-sc_require_config_h \
   autogen.sh
diff --git a/Makefile.maint b/Makefile.maint
index 5758215..b7bb680 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -135,6 +135,21 @@ sc_require_config_h:
 	else :;								\
 	fi

+# You must include <config.h> before including any other header file.
+sc_require_config_h_first:
+	@if $(VC_LIST_EXCEPT) | grep '\.c$$' > /dev/null; then		\
+	  fail=0;							\
+	  for i in $$($(VC_LIST_EXCEPT) | grep '\.c$$'); do		\
+	    grep '^# *include\>' $$i | sed 1q				\
+		| grep '^# *include <config\.h>' > /dev/null		\
+	      || { echo $$i; fail=1; };					\
+	  done;								\
+	  test $$fail = 1 &&						\
+	    { echo '$(ME): the above files include some other header'	\
+		'before <config.h>' 1>&2; exit 1; } || :;		\
+	else :;								\
+	fi
+
 # To use this "command" macro, you must first define two shell variables:
 # h: the header, enclosed in <> or ""
 # re: a regular expression that matches IFF something provided by $h is used.
diff --git a/qemud/Makefile.am b/qemud/Makefile.am
index b8dae88..28fd84a 100644
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -33,11 +33,16 @@ EXTRA_DIST =						\

 if RPCGEN
 SUFFIXES = .x
+# The perl -ne subshell ensures that remote_protocol.c ends up
+# including <config.h> before "remote_protocol.h".
 .x.c:
-	rm -f $@ $ -t $ -t2
+	rm -f $@ $ -t $ -t1 $ -t2
 	rpcgen -c -o $ -t $<
+	(echo '#include <config.h>';			\
+	 perl -ne '/^#include <config.h>/ or print' $ -t) > $ -t1
 if GLIBC_RPCGEN
-	perl -w rpcgen_fix.pl $ -t > $ -t2
+	perl -w rpcgen_fix.pl $ -t1 > $ -t2
+	rm $ -t1
 	chmod 444 $ -t2
 	mv $ -t2 $@
 endif
diff --git a/qemud/remote_protocol.c b/qemud/remote_protocol.c
index ec8e653..cbd722d 100644
--- a/qemud/remote_protocol.c
+++ b/qemud/remote_protocol.c
@@ -1,10 +1,10 @@
+#include <config.h>
 /*
  * Please do not edit this file.
  * It was generated using rpcgen.
  */

 #include "remote_protocol.h"
-#include <config.h>
 #include "internal.h"
 #include <arpa/inet.h>

--
1.6.1.rc2.316.geb2f0


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