[PATCH] Verify warning flags for CC_FOR_BUILD compiler

[PATCH] Verify warning flags for CC_FOR_BUILD compiler

[Vlad Zakharov]

Current commit introduces ac_cpp_for_build,
GCC_WARN_CFLAGS_FOR_BUILD, WARN_CFLAGS_FOR_BUILD and
AM_CFLAGS_FOR_BUILD variables and defines
AC_EGREP_CPP_FOR_BUILD macro.

This macro is used to verify compiler for build (CC_FOR_BUILD)
to be compatible with different flags and options, i. e.
verify if CC_FOR_BUILD compiler is suitable to use
WARN_CFLAGS_FOR_BUILD flags.

When cross-compiling we have different compiler CC and
CC_FOR_BUILD. But earlier AM_CFLAGS were used with both CC and
CC_FOR_BUILD compiler.

AM_CFLAGS are set up as WARN_CFLAGS + ZLIBINC.
In it's turn WARN_CFLAGS come up from GCC_WARN_CFLAGS.

Configure script verified GCC_WARN_CFLAGS to be compatible only
with CC compiler even though in fact these flags were also
passed to CC_FOR_BUILD. Such logic leads to some errors, i. e.
when we have old CC_FOR_BUILD compiler that is not suitable to
use some of GCC_WARN_CFLAGS.

To fix it corresponding variables and macro were added.
AM_CFLAGS_FOR_BUILD are passed now to CC_FOR_BUILD compiler
instead of AM_CFLAGS.

Signed-off-by: Vlad Zakharov <vzakhar@synopsys.com>
---
 bfd/ChangeLog        |  7 +++++++
 bfd/warning.m4       | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
 binutils/ChangeLog   |  5 +++++
 binutils/Makefile.am | 10 ++++++----
 4 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 6863e3a..74fa60a 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,10 @@
+2016-09-07  Vlad Zakharov  <vzakhar@synopsys.com>
+
+	* configure.ac (AC_EGREP_CPP_FOR_BUILD): Introduce macro 
+	to verify CC_FOR_BUILD compiler.
+	(AM_BINUTILS_WARNINGS):	Introduce ac_cpp_for_build variable
+	and add CC_FOR_BUILD compiler checks. 
+
 2016-09-06  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR ld/20550
diff --git a/bfd/warning.m4 b/bfd/warning.m4
index 4c5b55d..3d7fdbb 100644
--- a/bfd/warning.m4
+++ b/bfd/warning.m4
@@ -17,15 +17,37 @@ dnl along with this program; see the file COPYING3.  If not see
 dnl <http://www.gnu.org/licenses/>.
 dnl
 
+# AC_EGREP_CPP_FOR_BUILD(PATTERN, PROGRAM,
+#              [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND])
+# ------------------------------------------------------
+AC_DEFUN([AC_EGREP_CPP_FOR_BUILD],
+[AC_LANG_PREPROC_REQUIRE()dnl
+AC_REQUIRE([AC_PROG_EGREP])dnl
+AC_LANG_CONFTEST([AC_LANG_SOURCE([[$2]])])
+AS_IF([dnl eval is necessary to expand ac_cpp.
+dnl Ultrix and Pyramid sh refuse to redirect output of eval, so use subshell.
+(eval "$ac_cpp_for_build conftest.$ac_ext") 2>&AS_MESSAGE_LOG_FD |
+dnl Quote $1 to prevent m4 from eating character classes
+  $EGREP "[$1]" >/dev/null 2>&1],
+  [$3],
+  [$4])
+rm -f conftest*
+])# AC_EGREP_CPP_FOR_BUILD
+
+
 AC_DEFUN([AM_BINUTILS_WARNINGS],[
 # Set the 'development' global.
 . $srcdir/../bfd/development.sh
 
+# Set acp_cpp_for_build variable
+ac_cpp_for_build="$CC_FOR_BUILD -E $CPPFLAGS_FOR_BUILD"
+
 # Default set of GCC warnings to enable.
 GCC_WARN_CFLAGS="-W -Wall -Wstrict-prototypes -Wmissing-prototypes"
+GCC_WARN_CFLAGS_FOR_BUILD="-W -Wall -Wstrict-prototypes -Wmissing-prototypes"
 
 # Add -Wshadow if the compiler is a sufficiently recent version of GCC.
-AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
+AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_asdfasdfWARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
 
 # Add -Wstack-usage if the compiler is a sufficiently recent version of GCC.
 AC_EGREP_CPP([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wstack-usage=262144")
@@ -34,6 +56,14 @@ AC_EGREP_CPP([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wstack-usa
 WARN_WRITE_STRINGS=""
 AC_EGREP_CPP([^[0-3]$],[__GNUC__],,WARN_WRITE_STRINGS="-Wwrite-strings")
 
+# Verify CC_FOR_BUILD to be compatible with waring flags
+
+# Add -Wshadow if the compiler is a sufficiently recent version of GCC.
+AC_EGREP_CPP_FOR_BUILD([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wshadow")
+
+# Add -Wstack-usage if the compiler is a sufficiently recent version of GCC.
+AC_EGREP_CPP_FOR_BUILD([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wstack-usage=262144")
+
 AC_ARG_ENABLE(werror,
   [  --enable-werror         treat compile warnings as errors],
   [case "${enableval}" in
@@ -47,6 +77,7 @@ case "${host}" in
   *-*-mingw32*)
     if test "${GCC}" = yes -a -z "${ERROR_ON_WARNING}" ; then
       GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wno-format"
+      GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wno-format"
     fi
     ;;
   *) ;;
@@ -60,25 +91,32 @@ fi
 NO_WERROR=
 if test "${ERROR_ON_WARNING}" = yes ; then
     GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Werror"
+    GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Werror"
     NO_WERROR="-Wno-error"
 fi
 
 if test "${GCC}" = yes ; then
   WARN_CFLAGS="${GCC_WARN_CFLAGS}"
+  WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD}"
 fi
 
 AC_ARG_ENABLE(build-warnings,
 [  --enable-build-warnings enable build-time compiler warnings],
 [case "${enableval}" in
-  yes)	WARN_CFLAGS="${GCC_WARN_CFLAGS}";;
+  yes)	WARN_CFLAGS="${GCC_WARN_CFLAGS}" 
+        WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD}";;
   no)	if test "${GCC}" = yes ; then
 	  WARN_CFLAGS="-w"
+      WARN_CFLAGS_FOR_BUILD="-w" 
 	fi;;
   ,*)   t=`echo "${enableval}" | sed -e "s/,/ /g"`
-        WARN_CFLAGS="${GCC_WARN_CFLAGS} ${t}";;
+        WARN_CFLAGS="${GCC_WARN_CFLAGS} ${t}"
+        WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD} ${t}";;
   *,)   t=`echo "${enableval}" | sed -e "s/,/ /g"`
-        WARN_CFLAGS="${t} ${GCC_WARN_CFLAGS}";;
-  *)    WARN_CFLAGS=`echo "${enableval}" | sed -e "s/,/ /g"`;;
+        WARN_CFLAGS="${t} ${GCC_WARN_CFLAGS}"
+        WARN_CFLAGS_FOR_BUILD="${t} ${GCC_WARN_CFLAGS_FOR_BUILD}";;
+  *)    WARN_CFLAGS=`echo "${enableval}" | sed -e "s/,/ /g"`
+        WARN_CFLAGS_FOR_BUILD=`echo "${enableval}" | sed -e "s/,/ /g"`;;
 esac])
 
 if test x"$silent" != x"yes" && test x"$WARN_CFLAGS" != x""; then
@@ -86,6 +124,7 @@ if test x"$silent" != x"yes" && test x"$WARN_CFLAGS" != x""; then
 fi
 
 AC_SUBST(WARN_CFLAGS)
+AC_SUBST(WARN_CFLAGS_FOR_BUILD)
 AC_SUBST(NO_WERROR)
      AC_SUBST(WARN_WRITE_STRINGS)
 ])
diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 4b3a746..2884f18 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,8 @@
+2016-09-07  Vlad Zakharov  <vzakhar@synopsys.com>
+
+	* Makefile.am: Replace AM_CLFAGS with AM_CFLAGS_FOR_BUILD
+	when building with CC_FOR_BUILD compiler.
+	
 2016-09-06  Nick Clifton  <nickc@redhat.com>
 
 	* readelf.c (request_dump_bynumber): Only call memcpy if
diff --git a/binutils/Makefile.am b/binutils/Makefile.am
index 18af2c8..ef17914 100644
--- a/binutils/Makefile.am
+++ b/binutils/Makefile.am
@@ -47,8 +47,10 @@ ZLIB = @zlibdir@ -lz
 ZLIBINC = @zlibinc@
 
 WARN_CFLAGS = @WARN_CFLAGS@
+WARN_CFLAGS_FOR_BUILD = @WARN_CFLAGS_FOR_BUILD@
 NO_WERROR = @NO_WERROR@
 AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
+AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
 LIBICONV = @LIBICONV@
 
 # these two are almost the same program
@@ -305,17 +307,17 @@ sysinfo$(EXEEXT_FOR_BUILD): sysinfo.@OBJEXT@ syslex_wrap.@OBJEXT@
 	$(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@ sysinfo.@OBJEXT@ syslex_wrap.@OBJEXT@
 
 syslex_wrap.@OBJEXT@: syslex_wrap.c syslex.c sysinfo.h config.h
-	$(CC_FOR_BUILD) -c -I. -I$(srcdir) $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/syslex_wrap.c
+	$(CC_FOR_BUILD) -c -I. -I$(srcdir) $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/syslex_wrap.c
 
 sysinfo.@OBJEXT@: sysinfo.c
 	if [ -r sysinfo.c ]; then \
-	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) sysinfo.c ; \
+	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR) sysinfo.c ; \
 	else \
-	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/sysinfo.c ; \
+	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/sysinfo.c ; \
 	fi
 
 bin2c$(EXEEXT_FOR_BUILD): bin2c.c
-	$(CC_FOR_BUILD) -o $@ $(AM_CPPFLAGS) $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $(srcdir)/bin2c.c
+	$(CC_FOR_BUILD) -o $@ $(AM_CPPFLAGS) $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $(srcdir)/bin2c.c
 
 embedspu: embedspu.sh Makefile
 	awk '/^program_transform_name=/ {print "program_transform_name=\"$(program_transform_name)\""; next} {print}' < $< > $@
-- 
2.5.5

Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler

[Vlad Zakharov]

Hi Nick, all, 

Please treat this message as a polite reminder to review the patch.

It fixes a common issue discussed earlier in binuilts mailing list:
https://sourceware.org/ml/binutils/2016-08/msg00117.html
and prevents build failures of binutils.

Thanks.

On Wed, 2016-09-07 at 18:18 +0300, Vlad Zakharov wrote:
> Current commit introduces ac_cpp_for_build,
> GCC_WARN_CFLAGS_FOR_BUILD, WARN_CFLAGS_FOR_BUILD and
> AM_CFLAGS_FOR_BUILD variables and defines
> AC_EGREP_CPP_FOR_BUILD macro.
> 
> This macro is used to verify compiler for build (CC_FOR_BUILD)
> to be compatible with different flags and options, i. e.
> verify if CC_FOR_BUILD compiler is suitable to use
> WARN_CFLAGS_FOR_BUILD flags.
> 
> When cross-compiling we have different compiler CC and
> CC_FOR_BUILD. But earlier AM_CFLAGS were used with both CC and
> CC_FOR_BUILD compiler.
> 
> AM_CFLAGS are set up as WARN_CFLAGS + ZLIBINC.
> In it's turn WARN_CFLAGS come up from GCC_WARN_CFLAGS.
> 
> Configure script verified GCC_WARN_CFLAGS to be compatible only
> with CC compiler even though in fact these flags were also
> passed to CC_FOR_BUILD. Such logic leads to some errors, i. e.
> when we have old CC_FOR_BUILD compiler that is not suitable to
> use some of GCC_WARN_CFLAGS.
> 
> To fix it corresponding variables and macro were added.
> AM_CFLAGS_FOR_BUILD are passed now to CC_FOR_BUILD compiler
> instead of AM_CFLAGS.
> 
> Signed-off-by: Vlad Zakharov <vzakhar@synopsys.com>
> ---
>  bfd/ChangeLog        |  7 +++++++
>  bfd/warning.m4       | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
>  binutils/ChangeLog   |  5 +++++
>  binutils/Makefile.am | 10 ++++++----
>  4 files changed, 62 insertions(+), 9 deletions(-)
> 
> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> index 6863e3a..74fa60a 100644
> --- a/bfd/ChangeLog
> +++ b/bfd/ChangeLog
> @@ -1,3 +1,10 @@
> +2016-09-07  Vlad Zakharov  <vzakhar@synopsys.com>
> +
> +	* configure.ac (AC_EGREP_CPP_FOR_BUILD): Introduce macro 
> +	to verify CC_FOR_BUILD compiler.
> +	(AM_BINUTILS_WARNINGS):	Introduce ac_cpp_for_build variable
> +	and add CC_FOR_BUILD compiler checks. 
> +
>  2016-09-06  H.J. Lu  <hongjiu.lu@intel.com>
>  
>  	PR ld/20550
> diff --git a/bfd/warning.m4 b/bfd/warning.m4
> index 4c5b55d..3d7fdbb 100644
> --- a/bfd/warning.m4
> +++ b/bfd/warning.m4
> @@ -17,15 +17,37 @@ dnl along with this program; see the file COPYING3.  If not see
>  dnl <http://www.gnu.org/licenses/>.
>  dnl
>  
> +# AC_EGREP_CPP_FOR_BUILD(PATTERN, PROGRAM,
> +#              [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND])
> +# ------------------------------------------------------
> +AC_DEFUN([AC_EGREP_CPP_FOR_BUILD],
> +[AC_LANG_PREPROC_REQUIRE()dnl
> +AC_REQUIRE([AC_PROG_EGREP])dnl
> +AC_LANG_CONFTEST([AC_LANG_SOURCE([[$2]])])
> +AS_IF([dnl eval is necessary to expand ac_cpp.
> +dnl Ultrix and Pyramid sh refuse to redirect output of eval, so use subshell.
> +(eval "$ac_cpp_for_build conftest.$ac_ext") 2>&AS_MESSAGE_LOG_FD |
> +dnl Quote $1 to prevent m4 from eating character classes
> +  $EGREP "[$1]" >/dev/null 2>&1],
> +  [$3],
> +  [$4])
> +rm -f conftest*
> +])# AC_EGREP_CPP_FOR_BUILD
> +
> +
>  AC_DEFUN([AM_BINUTILS_WARNINGS],[
>  # Set the 'development' global.
>  . $srcdir/../bfd/development.sh
>  
> +# Set acp_cpp_for_build variable
> +ac_cpp_for_build="$CC_FOR_BUILD -E $CPPFLAGS_FOR_BUILD"
> +
>  # Default set of GCC warnings to enable.
>  GCC_WARN_CFLAGS="-W -Wall -Wstrict-prototypes -Wmissing-prototypes"
> +GCC_WARN_CFLAGS_FOR_BUILD="-W -Wall -Wstrict-prototypes -Wmissing-prototypes"
>  
>  # Add -Wshadow if the compiler is a sufficiently recent version of GCC.
> -AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
> +AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_asdfasdfWARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
>  
>  # Add -Wstack-usage if the compiler is a sufficiently recent version of GCC.
>  AC_EGREP_CPP([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wstack-usage=262144")
> @@ -34,6 +56,14 @@ AC_EGREP_CPP([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wstack-usa
>  WARN_WRITE_STRINGS=""
>  AC_EGREP_CPP([^[0-3]$],[__GNUC__],,WARN_WRITE_STRINGS="-Wwrite-strings")
>  
> +# Verify CC_FOR_BUILD to be compatible with waring flags
> +
> +# Add -Wshadow if the compiler is a sufficiently recent version of GCC.
> +AC_EGREP_CPP_FOR_BUILD([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wshadow")
> +
> +# Add -Wstack-usage if the compiler is a sufficiently recent version of GCC.
> +AC_EGREP_CPP_FOR_BUILD([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wstack-
> usage=262144")
> +
>  AC_ARG_ENABLE(werror,
>    [  --enable-werror         treat compile warnings as errors],
>    [case "${enableval}" in
> @@ -47,6 +77,7 @@ case "${host}" in
>    *-*-mingw32*)
>      if test "${GCC}" = yes -a -z "${ERROR_ON_WARNING}" ; then
>        GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wno-format"
> +      GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wno-format"
>      fi
>      ;;
>    *) ;;
> @@ -60,25 +91,32 @@ fi
>  NO_WERROR=
>  if test "${ERROR_ON_WARNING}" = yes ; then
>      GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Werror"
> +    GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Werror"
>      NO_WERROR="-Wno-error"
>  fi
>  
>  if test "${GCC}" = yes ; then
>    WARN_CFLAGS="${GCC_WARN_CFLAGS}"
> +  WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD}"
>  fi
>  
>  AC_ARG_ENABLE(build-warnings,
>  [  --enable-build-warnings enable build-time compiler warnings],
>  [case "${enableval}" in
> -  yes)	WARN_CFLAGS="${GCC_WARN_CFLAGS}";;
> +  yes)	WARN_CFLAGS="${GCC_WARN_CFLAGS}" 
> +        WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD}";;
>    no)	if test "${GCC}" = yes ; then
>  	  WARN_CFLAGS="-w"
> +      WARN_CFLAGS_FOR_BUILD="-w" 
>  	fi;;
>    ,*)   t=`echo "${enableval}" | sed -e "s/,/ /g"`
> -        WARN_CFLAGS="${GCC_WARN_CFLAGS} ${t}";;
> +        WARN_CFLAGS="${GCC_WARN_CFLAGS} ${t}"
> +        WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD} ${t}";;
>    *,)   t=`echo "${enableval}" | sed -e "s/,/ /g"`
> -        WARN_CFLAGS="${t} ${GCC_WARN_CFLAGS}";;
> -  *)    WARN_CFLAGS=`echo "${enableval}" | sed -e "s/,/ /g"`;;
> +        WARN_CFLAGS="${t} ${GCC_WARN_CFLAGS}"
> +        WARN_CFLAGS_FOR_BUILD="${t} ${GCC_WARN_CFLAGS_FOR_BUILD}";;
> +  *)    WARN_CFLAGS=`echo "${enableval}" | sed -e "s/,/ /g"`
> +        WARN_CFLAGS_FOR_BUILD=`echo "${enableval}" | sed -e "s/,/ /g"`;;
>  esac])
>  
>  if test x"$silent" != x"yes" && test x"$WARN_CFLAGS" != x""; then
> @@ -86,6 +124,7 @@ if test x"$silent" != x"yes" && test x"$WARN_CFLAGS" != x""; then
>  fi
>  
>  AC_SUBST(WARN_CFLAGS)
> +AC_SUBST(WARN_CFLAGS_FOR_BUILD)
>  AC_SUBST(NO_WERROR)
>       AC_SUBST(WARN_WRITE_STRINGS)
>  ])
> diff --git a/binutils/ChangeLog b/binutils/ChangeLog
> index 4b3a746..2884f18 100644
> --- a/binutils/ChangeLog
> +++ b/binutils/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-09-07  Vlad Zakharov  <vzakhar@synopsys.com>
> +
> +	* Makefile.am: Replace AM_CLFAGS with AM_CFLAGS_FOR_BUILD
> +	when building with CC_FOR_BUILD compiler.
> +	
>  2016-09-06  Nick Clifton  <nickc@redhat.com>
>  
>  	* readelf.c (request_dump_bynumber): Only call memcpy if
> diff --git a/binutils/Makefile.am b/binutils/Makefile.am
> index 18af2c8..ef17914 100644
> --- a/binutils/Makefile.am
> +++ b/binutils/Makefile.am
> @@ -47,8 +47,10 @@ ZLIB = @zlibdir@ -lz
>  ZLIBINC = @zlibinc@
>  
>  WARN_CFLAGS = @WARN_CFLAGS@
> +WARN_CFLAGS_FOR_BUILD = @WARN_CFLAGS_FOR_BUILD@
>  NO_WERROR = @NO_WERROR@
>  AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
>  LIBICONV = @LIBICONV@
>  
>  # these two are almost the same program
> @@ -305,17 +307,17 @@ sysinfo$(EXEEXT_FOR_BUILD): sysinfo.@OBJEXT@ syslex_wrap.@OBJEXT@
>  	$(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@ sysinfo.@OBJEXT@ syslex_wrap.@OBJEXT@
>  
>  syslex_wrap.@OBJEXT@: syslex_wrap.c syslex.c sysinfo.h config.h
> -	$(CC_FOR_BUILD) -c -I. -I$(srcdir) $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/syslex_wrap.c
> +	$(CC_FOR_BUILD) -c -I. -I$(srcdir) $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR)
> $(srcdir)/syslex_wrap.c
>  
>  sysinfo.@OBJEXT@: sysinfo.c
>  	if [ -r sysinfo.c ]; then \
> -	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) sysinfo.c ; \
> +	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR) sysinfo.c ; \
>  	else \
> -	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/sysinfo.c ; \
> +	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/sysinfo.c ; \
>  	fi
>  
>  bin2c$(EXEEXT_FOR_BUILD): bin2c.c
> -	$(CC_FOR_BUILD) -o $@ $(AM_CPPFLAGS) $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $(srcdir)/bin2c.c
> +	$(CC_FOR_BUILD) -o $@ $(AM_CPPFLAGS) $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD)
> $(srcdir)/bin2c.c
>  
>  embedspu: embedspu.sh Makefile
>  	awk '/^program_transform_name=/ {print "program_transform_name=\"$(program_transform_name)\""; next} {print}'
> < $< > $@
-- 
Best regards,
Vlad Zakharov <vzakhar@synopsys.com>

Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler

[Nick Clifton]

Hi Vlad,

> Please treat this message as a polite reminder to review the patch.

Oops- sorry for dropping the ball on this one.

I have a couple of questions about the patch:

>>  # Add -Wshadow if the compiler is a sufficiently recent version of GCC.
>> -AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
>> +AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_asdfasdfWARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")

What is this for ?  Why is the change being made and what is the significance of 'asdfasdf' ?

Other than that though the patch looks good to me.  I am withholding approval 
pending an answer to the above question, (which I suspect might turn out to be
a typo), but I think that we are almost there.

One other small point - it is easier for us if you include the proposed ChangeLog
entries for your patch as plain text rather then context diffs.  This is because
the diff almost never applies directly to the sources, since the changelogs change
so frequently.

Oh, and it is good practice to omit diffs for auto-generated files like Makefile.in,
as you have done, but you should mention in the changelog entry that the file has
been regenerated.  Ie:

2016-09-07  Vlad Zakharov  <vzakhar@synopsys.com>

	* Makefile.am: Replace AM_CLFAGS with AM_CFLAGS_FOR_BUILD
	when building with CC_FOR_BUILD compiler.
	* Makefile.in: Regenerate.

Cheers
  Nick

 	

Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler

[Vlad Zakharov]

Hi Nick,

On Mon, 2016-09-26 at 10:32 +0100, Nick Clifton wrote:
> Hi Vlad,
> 
> > 
> > Please treat this message as a polite reminder to review the patch.
> 
> Oops- sorry for dropping the ball on this one.
> 
> I have a couple of questions about the patch:
> 
> > 
> > > 
> > >  # Add -Wshadow if the compiler is a sufficiently recent version of GCC.
> > > -AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
> > > +AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_asdfasdfWARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
> 
> What is this for ?  Why is the change being made and what is the significance of 'asdfasdf' ?
> 
> Other than that though the patch looks good to me.  I am withholding approval 
> pending an answer to the above question, (which I suspect might turn out to be
> a typo), but I think that we are almost there.

Oh, sorry for that, it is really a typo.
Of course there shouldn't be any updates at that line. It's just my silly
mistake.

> 
> One other small point - it is easier for us if you include the proposed ChangeLog
> entries for your patch as plain text rather then context diffs.  This is because
> the diff almost never applies directly to the sources, since the changelogs change
> so frequently.
> 
> Oh, and it is good practice to omit diffs for auto-generated files like Makefile.in,
> as you have done, but you should mention in the changelog entry that the file has
> been regenerated.  Ie:
> 
> 2016-09-07  Vlad Zakharov  <vzakhar@synopsys.com>
> 
> 	* Makefile.am: Replace AM_CLFAGS with AM_CFLAGS_FOR_BUILD
> 	when building with CC_FOR_BUILD compiler.
> 	* Makefile.in: Regenerate.
> 
> Cheers
>   Nick
> 
>  	

I have taken note of your comments about ChangeLogs and auto-generated files.
Should I resend my patch with these minor
updates?

Thanks.

-- 
Best regards,
Vlad Zakharov <vzakhar@synopsys.com>

Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler

[Nick Clifton]

Hi Vlad,

> I have taken note of your comments about ChangeLogs and auto-generated files.
> Should I resend my patch with these minor updates?

Yes please, just for completeness sake.

BTW do you have commit access, or would you like me to apply the patch for you ?

Cheers
  Nick

Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler

[Vlad Zakharov]

Hi Nick,

On Mon, 2016-09-26 at 12:53 +0100, Nick Clifton wrote:
> Hi Vlad,
> 
> > 
> > I have taken note of your comments about ChangeLogs and auto-generated files.
> > Should I resend my patch with these minor updates?
> 
> Yes please, just for completeness sake.

OK, I'll send the updated patch ASAP. 

> 
> BTW do you have commit access, or would you like me to apply the patch for you ?
> 
> Cheers
>   Nick
> 

I suppose I have not, so I would like you to apply the patch.

Thanks.

-- 
Best regards,
Vlad Zakharov <vzakhar@synopsys.com>

Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler

[Cupertino Miranda]

Hi Nick,

As soon as the patch gets approved either me or Claudiu can apply the
patch to Vlad.

Best regards,
Cupertino

On 09/26/2016 12:58 PM, Vlad Zakharov wrote:
> Hi Nick,
>
> On Mon, 2016-09-26 at 12:53 +0100, Nick Clifton wrote:
>> Hi Vlad,
>>
>>> I have taken note of your comments about ChangeLogs and auto-generated files.
>>> Should I resend my patch with these minor updates?
>> Yes please, just for completeness sake.
> OK, I'll send the updated patch ASAP. 
>
>> BTW do you have commit access, or would you like me to apply the patch for you ?
>>
>> Cheers
>>   Nick
>>
> I suppose I have not, so I would like you to apply the patch.
>
> Thanks.
>