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

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



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 :|


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