[libvirt] [PATCH] Fix _FILE_OFFSET_BITS re-definition
# 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"
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
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
On Wed, Dec 17, 2008 at 10:55:41AM +0100, Jim Meyering wrote:
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.
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@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@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>
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 :|
"Daniel P. Berrange" <berrange@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.
participants (4)
-
Daniel P. Berrange -
Jim Meyering -
John Levon -
john.levon＠sun.com