Patchwork scripts: add a guard macro in generated .h files

login
register
mail settings
Submitter Tristan Gingold
Date Feb. 7, 2011, 3:21 p.m.
Message ID <1297092100-19171-1-git-send-email-gingold@adacore.com>
Download mbox | patch
Permalink /patch/82220/
State New
Headers show

Comments

Tristan Gingold - Feb. 7, 2011, 3:21 p.m.
To avoid redefinition warnings.

Signed-off-by: Tristan Gingold <gingold@adacore.com>
---
 rules.mak             |    2 +-
 scripts/create_config |   12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletions(-)
Aurelien Jarno - Feb. 20, 2011, 6:14 p.m.
On Mon, Feb 07, 2011 at 04:21:40PM +0100, Tristan Gingold wrote:
> To avoid redefinition warnings.
> 
> Signed-off-by: Tristan Gingold <gingold@adacore.com>
> ---
>  rules.mak             |    2 +-
>  scripts/create_config |   12 ++++++++++++
>  2 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/rules.mak b/rules.mak
> index ed59c9e..fb38f96 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -57,7 +57,7 @@ find-in-path = $(if $(find-string /, $1), \
>  	@test -f $@ || cp $< $@
>  
>  %.h-timestamp: %.mak
> -	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config < $< > $@, "  GEN   $*.h")
> +	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config $* < $< > $@, "  GEN   $*.h")
>  	@cmp $@ $*.h >/dev/null 2>&1 || cp $@ $*.h
>  
>  # will delete the target of a rule if commands exit with a nonzero exit status
> diff --git a/scripts/create_config b/scripts/create_config
> index 0098e68..0302eab 100755
> --- a/scripts/create_config
> +++ b/scripts/create_config
> @@ -1,7 +1,17 @@
>  #!/bin/sh
>  
> +if [ $# -ne 1 ]; then
> +  echo "Usage: $0 filename" 2>&1
> +  exit 1
> +fi
> +
> +cond_macro=`echo $1 | tr a-z- A-Z_`_H
> +
>  echo "/* Automatically generated by create_config - do not modify */"
>  
> +echo "#ifndef $cond_macro"
> +echo "#define $cond_macro"
> +
>  while read line; do
>  
>  case $line in
> @@ -101,3 +111,5 @@ case $line in
>  esac
>  
>  done # read
> +
> +echo "#endif /* $cond_macro */"

config-host.h only contains #define entries, and GCC doesn't choke when
as long as the definitions are the same. What is the use case of this
patch?
Tristan Gingold - Feb. 21, 2011, 6:42 a.m.
On Feb 20, 2011, at 7:14 PM, Aurelien Jarno wrote:

> On Mon, Feb 07, 2011 at 04:21:40PM +0100, Tristan Gingold wrote:
>> To avoid redefinition warnings.
> config-host.h only contains #define entries, and GCC doesn't choke when
> as long as the definitions are the same. What is the use case of this
> patch? 

Mostly a style issue.  It is common to always protect header files against multiple inclusion, unless the header is meant to be
included several times (which is not the case for these config files).  I think this is a good practice.

I once got redefinition warnings for macros in config-host.h, but I agree that adding the guard macro doesn't fix this issue.

If you agree that adding guards is harmless and good style, I can change the comment.

Tristan.
Paolo Bonzini - Feb. 21, 2011, 8:14 a.m.
On 02/21/2011 07:42 AM, Tristan Gingold wrote:
> Mostly a style issue.  It is common to always protect header files
> against multiple inclusion, unless the header is meant to be included
> several times (which is not the case for these config files).  I
> think this is a good practice.

Traditionally, autoconf's config.h headers have no guards either.  This 
can indeed cause some troubles.  However, it also helps highlighting 
poor practices, such as libraries installing a config.h file including 
it from a public header.

> I once got redefinition warnings for macros in config-host.h, but I
> agree that adding the guard macro doesn't fix this issue.

config-host.h should always be the first included header (we are poor at 
this) and its macros should never conflict with anything else.  I think 
we should rather fix the problems you've seen with config-host.h, if you 
can still reproduce them.

Paolo
Tristan Gingold - Feb. 21, 2011, 6:13 p.m.
On Feb 21, 2011, at 9:14 AM, Paolo Bonzini wrote:

> On 02/21/2011 07:42 AM, Tristan Gingold wrote:
>> Mostly a style issue.  It is common to always protect header files
>> against multiple inclusion, unless the header is meant to be included
>> several times (which is not the case for these config files).  I
>> think this is a good practice.
> 
> Traditionally, autoconf's config.h headers have no guards either.  This can indeed cause some troubles.  However, it also helps highlighting poor practices, such as libraries installing a config.h file including it from a public header.
> 
>> I once got redefinition warnings for macros in config-host.h, but I
>> agree that adding the guard macro doesn't fix this issue.
> 
> config-host.h should always be the first included header (we are poor at this) and its macros should never conflict with anything else.  I think we should rather fix the problems you've seen with config-host.h, if you can still reproduce them.

Thank you.  I will investigate the next time I have the issue and the time.

Tristan.

Patch

diff --git a/rules.mak b/rules.mak
index ed59c9e..fb38f96 100644
--- a/rules.mak
+++ b/rules.mak
@@ -57,7 +57,7 @@  find-in-path = $(if $(find-string /, $1), \
 	@test -f $@ || cp $< $@
 
 %.h-timestamp: %.mak
-	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config < $< > $@, "  GEN   $*.h")
+	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config $* < $< > $@, "  GEN   $*.h")
 	@cmp $@ $*.h >/dev/null 2>&1 || cp $@ $*.h
 
 # will delete the target of a rule if commands exit with a nonzero exit status
diff --git a/scripts/create_config b/scripts/create_config
index 0098e68..0302eab 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -1,7 +1,17 @@ 
 #!/bin/sh
 
+if [ $# -ne 1 ]; then
+  echo "Usage: $0 filename" 2>&1
+  exit 1
+fi
+
+cond_macro=`echo $1 | tr a-z- A-Z_`_H
+
 echo "/* Automatically generated by create_config - do not modify */"
 
+echo "#ifndef $cond_macro"
+echo "#define $cond_macro"
+
 while read line; do
 
 case $line in
@@ -101,3 +111,5 @@  case $line in
 esac
 
 done # read
+
+echo "#endif /* $cond_macro */"