'yes' within the path sets wrong config variables
diff mbox

Message ID 1425394379-41448-1-git-send-email-benjamin.esquivel@linux.intel.com
State New
Headers show

Commit Message

Benjamin Esquivel March 3, 2015, 2:52 p.m. UTC
It seems that the 'AC_EGREP_CPP(yes...' example is quite popular
but being such a short word to grep it is likely to produce
false-positive matches with the path it is configured into.

The change is to use a more elaborated string to grep for.

Signed-off-by: Benjamin Esquivel <benjamin.esquivel@linux.intel.com>
---
 sysdeps/aarch64/configure                              | 4 ++--
 sysdeps/aarch64/configure.ac                           | 4 ++--
 sysdeps/arm/configure                                  | 4 ++--
 sysdeps/arm/configure.ac                               | 4 ++--
 sysdeps/mips/configure                                 | 4 ++--
 sysdeps/mips/configure.ac                              | 4 ++--
 sysdeps/nios2/configure                                | 4 ++--
 sysdeps/nios2/configure.ac                             | 4 ++--
 sysdeps/unix/sysv/linux/mips/configure                 | 4 ++--
 sysdeps/unix/sysv/linux/mips/configure.ac              | 4 ++--
 sysdeps/unix/sysv/linux/powerpc/powerpc64/configure    | 8 ++++----
 sysdeps/unix/sysv/linux/powerpc/powerpc64/configure.ac | 8 ++++----
 12 files changed, 28 insertions(+), 28 deletions(-)

Comments

Mike Frysinger March 3, 2015, 11:31 p.m. UTC | #1
On 03 Mar 2015 14:52, Benjamin Esquivel wrote:
> It seems that the 'AC_EGREP_CPP(yes...' example is quite popular
> but being such a short word to grep it is likely to produce
> false-positive matches with the path it is configured into.

change looks fine (except you need ChangeLog entries), but i wonder if this 
idiom is even the right one.  why aren't these all compile tests that do the 
standard "choke me" failure instead ?

autoconf-archive even has an off the shelf macro we could import:
ax_check_define.m4:AC_CHECK_DEFINE

then the code would be cleaner/easier to read:
	AC_CHECK_DEFINE([__AARCH64EB__], [
	  AC_DEFINE(HAVE_AARCH64_BE)
	  LIBC_CONFIG_VAR([default-abi], [lp64_be])
	], [
	  LIBC_CONFIG_VAR([default-abi], [lp64])
	])
this replaces all the code that is currently in sysdeps/aarch64/configure.ac.

it does mean the cache var changes names:
	libc_cv_aarch64_be -> ac_cv_defined___AARCH64EB__
but maybe we don't care ?  if that really is important to us, we could simply 
define our own macro based on that thatuses a different cache var name to our 
liking.

i haven't looked at the whole code base though to see whether this macro would 
be able to replace all of them ...
-mike
Joseph Myers March 4, 2015, 3:58 a.m. UTC | #2
On Tue, 3 Mar 2015, Benjamin Esquivel wrote:

> It seems that the 'AC_EGREP_CPP(yes...' example is quite popular
> but being such a short word to grep it is likely to produce
> false-positive matches with the path it is configured into.

What's your objection to the AC_PREPROC_IFELSE / #error approach suggested 
in <https://sourceware.org/ml/libc-alpha/2014-08/msg00414.html>?
Mike Frysinger March 4, 2015, 6 a.m. UTC | #3
On 04 Mar 2015 03:58, Joseph Myers wrote:
> On Tue, 3 Mar 2015, Benjamin Esquivel wrote:
> > It seems that the 'AC_EGREP_CPP(yes...' example is quite popular
> > but being such a short word to grep it is likely to produce
> > false-positive matches with the path it is configured into.
> 
> What's your objection to the AC_PREPROC_IFELSE / #error approach suggested 
> in <https://sourceware.org/ml/libc-alpha/2014-08/msg00414.html>?

that would be an improvement over the status quo, but i think AC_CHECK_DEFINE is 
even better
-mike
Benjamin Esquivel March 4, 2015, 4:04 p.m. UTC | #4
On Wed, 2015-03-04 at 01:00 -0500, Mike Frysinger wrote:
> On 04 Mar 2015 03:58, Joseph Myers wrote:
> > On Tue, 3 Mar 2015, Benjamin Esquivel wrote:
> > > It seems that the 'AC_EGREP_CPP(yes...' example is quite popular
> > > but being such a short word to grep it is likely to produce
> > > false-positive matches with the path it is configured into.
> > 
> > What's your objection to the AC_PREPROC_IFELSE / #error approach suggested 
> > in <https://sourceware.org/ml/libc-alpha/2014-08/msg00414.html>?
> 

I don't have an objection to either approaches. Semantically, it looks
to me that AC_CHECK_DEFINE is a better option. I'll give it a try.

> that would be an improvement over the status quo, but i think AC_CHECK_DEFINE is 
> even better


> -mike
Benjamin Esquivel March 5, 2015, 6:55 p.m. UTC | #5
On Wed, 2015-03-04 at 10:04 -0600, Benjamin Esquivel wrote:
> On Wed, 2015-03-04 at 01:00 -0500, Mike Frysinger wrote:
> > On 04 Mar 2015 03:58, Joseph Myers wrote:
> > > On Tue, 3 Mar 2015, Benjamin Esquivel wrote:
> > > > It seems that the 'AC_EGREP_CPP(yes...' example is quite popular
> > > > but being such a short word to grep it is likely to produce
> > > > false-positive matches with the path it is configured into.
> > > 
> > > What's your objection to the AC_PREPROC_IFELSE / #error approach suggested 
> > > in <https://sourceware.org/ml/libc-alpha/2014-08/msg00414.html>?
> > 
> 
> I don't have an objection to either approaches. Semantically, it looks
> to me that AC_CHECK_DEFINE is a better option. I'll give it a try.
> 
> > that would be an improvement over the status quo, but i think AC_CHECK_DEFINE is 
> > even better
> 

Now, the thing is that I have been unsuccessful at doing the reconfigure
required to get the 'configure' files out of the modified 'configure.ac'
files.

So far I've tried autoreconf, and make configure at the glibc root dir
and got errors.

Any input on how to reconf is appreciated.

> 
> > -mike
> 
>
Roland McGrath March 5, 2015, 7:01 p.m. UTC | #6
> Any input on how to reconf is appreciated.

If you configure with --enable-maintainer-mode it will set AUTOCONF to
something other than 'no'.  Then the makefiles will regenerate configure
scripts automatically when you do a 'make'.  That will only cover the ones
used in your configuration, which is enough to test that things are working
as you expect.  Then you can do 'make dist-prepare' to get all of them
regenerated.
Benjamin Esquivel March 5, 2015, 9:04 p.m. UTC | #7
On Thu, 2015-03-05 at 11:01 -0800, Roland McGrath wrote:
> > Any input on how to reconf is appreciated.
> 
> If you configure with --enable-maintainer-mode it will set AUTOCONF to
> something other than 'no'.  Then the makefiles will regenerate configure
> scripts automatically when you do a 'make'.  That will only cover the ones
> used in your configuration, which is enough to test that things are working
> as you expect.  Then you can do 'make dist-prepare' to get all of them
> regenerated.
Thanks, that worked fine. 

Now for the case where a macro coming from autoconf-archive is to be
added here, I presume you'd only place such macro into the aclocal.m4,
as I don't see an acinclude.m4 file nor an m4/ dir.

Please confirm :)
Roland McGrath March 5, 2015, 9:16 p.m. UTC | #8
> Now for the case where a macro coming from autoconf-archive is to be
> added here, I presume you'd only place such macro into the aclocal.m4,
> as I don't see an acinclude.m4 file nor an m4/ dir.

Indeed we do not use acinclude.m4 since we do not use Automake.

If it's something maintained elsewhere it would be best to put it into its
own file so that it's easy to copy that file verbatim from newer versions.
Then you can m4_include that from aclocal.m4.
Mike Frysinger March 5, 2015, 11:55 p.m. UTC | #9
On 05 Mar 2015 12:55, Benjamin Esquivel wrote:
> Now, the thing is that I have been unsuccessful at doing the reconfigure
> required to get the 'configure' files out of the modified 'configure.ac'
> files.
> 
> So far I've tried autoreconf, and make configure at the glibc root dir
> and got errors.
> 
> Any input on how to reconf is appreciated.

generally i do it by hand: run autoconf and pass it -I to the toplevel.
-mike

Patch
diff mbox

diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index 5bd355a..3bc5537 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -148,12 +148,12 @@  else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #ifdef __AARCH64EB__
-                      yes
+                      is_aarch64_be
                      #endif
 
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
-  $EGREP "yes" >/dev/null 2>&1; then :
+  $EGREP "is_aarch64_be" >/dev/null 2>&1; then :
   libc_cv_aarch64_be=yes
 else
   libc_cv_aarch64_be=no
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 7851dd4..6e92381 100644
--- a/sysdeps/aarch64/configure.ac
+++ b/sysdeps/aarch64/configure.ac
@@ -10,8 +10,8 @@  GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
 # the dynamic linker via %ifdef.
 AC_CACHE_CHECK([for big endian],
   [libc_cv_aarch64_be],
-  [AC_EGREP_CPP(yes,[#ifdef __AARCH64EB__
-                      yes
+  [AC_EGREP_CPP(is_aarch64_be,[#ifdef __AARCH64EB__
+                      is_aarch64_be
                      #endif
   ], libc_cv_aarch64_be=yes, libc_cv_aarch64_be=no)])
 if test $libc_cv_aarch64_be = yes; then
diff --git a/sysdeps/arm/configure b/sysdeps/arm/configure
index 45667cc..0b2ef11 100644
--- a/sysdeps/arm/configure
+++ b/sysdeps/arm/configure
@@ -150,12 +150,12 @@  else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #ifdef __ARM_PCS_VFP
-		      yes
+		      use_arm_pcs_vfp
 		     #endif
 
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
-  $EGREP "yes" >/dev/null 2>&1; then :
+  $EGREP "use_arm_pcs_vfp" >/dev/null 2>&1; then :
   libc_cv_arm_pcs_vfp=yes
 else
   libc_cv_arm_pcs_vfp=no
diff --git a/sysdeps/arm/configure.ac b/sysdeps/arm/configure.ac
index 002b8ef..e1746a7 100644
--- a/sysdeps/arm/configure.ac
+++ b/sysdeps/arm/configure.ac
@@ -16,8 +16,8 @@  dnl it.  Until we do, don't define it.
 # the dynamic linker via %ifdef.
 AC_CACHE_CHECK([whether the compiler is using the ARM hard-float ABI],
   [libc_cv_arm_pcs_vfp],
-  [AC_EGREP_CPP(yes,[#ifdef __ARM_PCS_VFP
-		      yes
+  [AC_EGREP_CPP(use_arm_pcs_vfp,[#ifdef __ARM_PCS_VFP
+		      use_arm_pcs_vfp
 		     #endif
   ], libc_cv_arm_pcs_vfp=yes, libc_cv_arm_pcs_vfp=no)])
 if test $libc_cv_arm_pcs_vfp = yes; then
diff --git a/sysdeps/mips/configure b/sysdeps/mips/configure
index 4e13248..f14af95 100644
--- a/sysdeps/mips/configure
+++ b/sysdeps/mips/configure
@@ -143,11 +143,11 @@  else
 /* end confdefs.h.  */
 dnl
 #ifdef __mips_nan2008
-yes
+use_mips_nan2008
 #endif
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
-  $EGREP "yes" >/dev/null 2>&1; then :
+  $EGREP "use_mips_nan2008" >/dev/null 2>&1; then :
   libc_cv_mips_nan2008=yes
 else
   libc_cv_mips_nan2008=no
diff --git a/sysdeps/mips/configure.ac b/sysdeps/mips/configure.ac
index bcbdaff..ad3057f 100644
--- a/sysdeps/mips/configure.ac
+++ b/sysdeps/mips/configure.ac
@@ -6,9 +6,9 @@  dnl position independent way.
 dnl AC_DEFINE(PI_STATIC_AND_HIDDEN)
 
 AC_CACHE_CHECK([whether the compiler is using the 2008 NaN encoding],
-  libc_cv_mips_nan2008, [AC_EGREP_CPP(yes, [dnl
+  libc_cv_mips_nan2008, [AC_EGREP_CPP(use_mips_nan2008, [dnl
 #ifdef __mips_nan2008
-yes
+use_mips_nan2008
 #endif], libc_cv_mips_nan2008=yes, libc_cv_mips_nan2008=no)])
 if test x$libc_cv_mips_nan2008 = xyes; then
   AC_DEFINE(HAVE_MIPS_NAN2008)
diff --git a/sysdeps/nios2/configure b/sysdeps/nios2/configure
index 14c8a3a..dde3814 100644
--- a/sysdeps/nios2/configure
+++ b/sysdeps/nios2/configure
@@ -142,12 +142,12 @@  else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #ifdef __nios2_big_endian__
-                      yes
+                      is_nios2_be
                      #endif
 
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
-  $EGREP "yes" >/dev/null 2>&1; then :
+  $EGREP "is_nios2_be" >/dev/null 2>&1; then :
   libc_cv_nios2_be=yes
 else
   libc_cv_nios2_be=no
diff --git a/sysdeps/nios2/configure.ac b/sysdeps/nios2/configure.ac
index f05f438..dc86399 100644
--- a/sysdeps/nios2/configure.ac
+++ b/sysdeps/nios2/configure.ac
@@ -4,8 +4,8 @@  GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
 # Nios II big endian is not yet supported.
 AC_CACHE_CHECK([for big endian],
   [libc_cv_nios2_be],
-  [AC_EGREP_CPP(yes,[#ifdef __nios2_big_endian__
-                      yes
+  [AC_EGREP_CPP(is_nios2_be,[#ifdef __nios2_big_endian__
+                      is_nios2_be
                      #endif
   ], libc_cv_nios2_be=yes, libc_cv_nios2_be=no)])
 if test $libc_cv_nios2_be = yes; then
diff --git a/sysdeps/unix/sysv/linux/mips/configure b/sysdeps/unix/sysv/linux/mips/configure
index 83f8b13..2b6cbee 100644
--- a/sysdeps/unix/sysv/linux/mips/configure
+++ b/sysdeps/unix/sysv/linux/mips/configure
@@ -387,11 +387,11 @@  else
 /* end confdefs.h.  */
 dnl
 #ifdef __mips_nan2008
-yes
+use_mips_nan2008
 #endif
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
-  $EGREP "yes" >/dev/null 2>&1; then :
+  $EGREP "use_mips_nan2008" >/dev/null 2>&1; then :
   libc_cv_mips_nan2008=yes
 else
   libc_cv_mips_nan2008=no
diff --git a/sysdeps/unix/sysv/linux/mips/configure.ac b/sysdeps/unix/sysv/linux/mips/configure.ac
index 5039ec9..1035f76 100644
--- a/sysdeps/unix/sysv/linux/mips/configure.ac
+++ b/sysdeps/unix/sysv/linux/mips/configure.ac
@@ -98,9 +98,9 @@  AC_COMPILE_IFELSE(
 LIBC_CONFIG_VAR([mips-mode-switch],[${libc_mips_mode_switch}])
 
 AC_CACHE_CHECK([whether the compiler is using the 2008 NaN encoding],
-  libc_cv_mips_nan2008, [AC_EGREP_CPP(yes, [dnl
+  libc_cv_mips_nan2008, [AC_EGREP_CPP(use_mips_nan2008, [dnl
 #ifdef __mips_nan2008
-yes
+use_mips_nan2008
 #endif], libc_cv_mips_nan2008=yes, libc_cv_mips_nan2008=no)])
 
 libc_mips_nan=
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/configure b/sysdeps/unix/sysv/linux/powerpc/powerpc64/configure
index 70bb18a..ffd9e3e 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/configure
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/configure
@@ -155,12 +155,12 @@  else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #if _CALL_ELF == 2
-                      yes
+                      use_ppc_elfv2_abi
                      #endif
 
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
-  $EGREP "yes" >/dev/null 2>&1; then :
+  $EGREP "use_ppc_elfv2_abi" >/dev/null 2>&1; then :
   libc_cv_ppc64_elfv2_abi=yes
 else
   libc_cv_ppc64_elfv2_abi=no
@@ -188,12 +188,12 @@  else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #ifdef _CALL_ELF
-                         yes
+                         is_def_call_elf
                        #endif
 
 _ACEOF
 if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
-  $EGREP "yes" >/dev/null 2>&1; then :
+  $EGREP "is_def_call_elf" >/dev/null 2>&1; then :
   libc_cv_ppc64_def_call_elf=yes
 else
   libc_cv_ppc64_def_call_elf=no
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/configure.ac b/sysdeps/unix/sysv/linux/powerpc/powerpc64/configure.ac
index 0822915..9a32fdd 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/configure.ac
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/configure.ac
@@ -6,8 +6,8 @@  LIBC_SLIBDIR_RTLDDIR([lib64], [lib64])
 # Define default-abi according to compiler flags.
 AC_CACHE_CHECK([whether the compiler is using the PowerPC64 ELFv2 ABI],
   [libc_cv_ppc64_elfv2_abi],
-  [AC_EGREP_CPP(yes,[#if _CALL_ELF == 2
-                      yes
+  [AC_EGREP_CPP(use_ppc_elfv2_abi,[#if _CALL_ELF == 2
+                      use_ppc_elfv2_abi
                      #endif
   ], libc_cv_ppc64_elfv2_abi=yes, libc_cv_ppc64_elfv2_abi=no)])
 if test $libc_cv_ppc64_elfv2_abi = yes; then
@@ -19,8 +19,8 @@  else
   # Compiler that do not support ELFv2 ABI does not define _CALL_ELF
   AC_CACHE_CHECK([whether the compiler defines _CALL_ELF],
     [libc_cv_ppc64_def_call_elf],
-    [AC_EGREP_CPP(yes,[#ifdef _CALL_ELF
-                         yes
+    [AC_EGREP_CPP(is_def_call_elf,[#ifdef _CALL_ELF
+                         is_def_call_elf
                        #endif
     ], libc_cv_ppc64_def_call_elf=yes, libc_cv_ppc64_def_call_elf=no)])
   if test $libc_cv_ppc64_def_call_elf = no; then