8:52 p.m.
# HG changeset patch
# User john.levon(a)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(a)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(a)virtualiron.com>
*/
+#include <config.h>
+
#include <stdio.h>
#include <stdlib.h>
-#include <config.h>
#include <libhal.h>
#include "node_device_conf.h"
3:55 a.m.
john.levon(a)sun.com wrote:
# HG changeset patch
# User john.levon(a)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(a)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(a)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(a)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(a)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(a)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
6:39 a.m.
On Wed, Dec 17, 2008 at 10:55:41AM +0100, Jim Meyering wrote:
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
One of my other patches fixes this last one.
so I've just written a new syntax-check rule to enforce this.
Cool, thanks!
regards
john
9:21 a.m.
On Wed, Dec 17, 2008 at 10:55:41AM +0100, Jim Meyering wrote:
john.levon(a)sun.com wrote:
> # HG changeset patch
> # User john.levon(a)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(a)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(a)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.
Yep, the examples shouldn't really need config.h at all,
using the
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(a)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.
ACK
---
.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(a)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(a)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>
Rather than filtering out the bogus 'config.h' from remote_protocol.c
in the Makefile.am rule, just kill this line from the original protocol
definition:
%#include <config.h>
Then the bogus placed include wouldn't be added in the first place.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:35 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
...
> 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
...
Rather than filtering out the bogus 'config.h' from
remote_protocol.c
in the Makefile.am rule, just kill this line from the original protocol
definition:
%#include <config.h>
Then the bogus placed include wouldn't be added in the first place.
Good idea.
Committed with that change, which induced the removal of
#include <config.h> from remote_protocol.h, too.
But that's fine, because that .h file never needed it in the first place.
5822
days inactive
5822
days old
4 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Jim Meyering -
John Levon -
john.levon＠sun.com