diff mbox

configure: Use strings command from cross development tools

Message ID 20180513085032.27773-1-sw@weilnetz.de (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Weil May 13, 2018, 8:50 a.m. UTC
This fixes cross builds for the (rare) case where cross binutils
but no native binutils are installed.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 configure | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Philippe Mathieu-Daudé May 13, 2018, 9:12 p.m. UTC | #1
On 05/13/2018 05:50 AM, Stefan Weil wrote:
> This fixes cross builds for the (rare) case where cross binutils
> but no native binutils are installed.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  configure | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 24c411e346..4f6ace1ed4 100755
> --- a/configure
> +++ b/configure
> @@ -517,6 +517,7 @@ objcopy="${OBJCOPY-${cross_prefix}objcopy}"
>  ld="${LD-${cross_prefix}ld}"
>  ranlib="${RANLIB-${cross_prefix}ranlib}"
>  nm="${NM-${cross_prefix}nm}"
> +strings="${STRINGS-${cross_prefix}strings}"
>  strip="${STRIP-${cross_prefix}strip}"
>  windres="${WINDRES-${cross_prefix}windres}"
>  pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
> @@ -1956,9 +1957,9 @@ int main(int argc, char *argv[]) {
>  EOF
>  
>  if compile_object ; then
> -    if strings -a $TMPO | grep -q BiGeNdIaN ; then
> +    if "$strings" -a $TMPO | grep -q BiGeNdIaN ; then
>          bigendian="yes"
> -    elif strings -a $TMPO | grep -q LiTtLeEnDiAn ; then
> +    elif "$strings" -a $TMPO | grep -q LiTtLeEnDiAn ; then
>          bigendian="no"
>      else
>          echo big/little test failed
>
Richard Henderson May 14, 2018, 3:26 p.m. UTC | #2
On 05/13/2018 01:50 AM, Stefan Weil wrote:
> This fixes cross builds for the (rare) case where cross binutils
> but no native binutils are installed.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  configure | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Eric Blake May 15, 2018, 3 p.m. UTC | #3
On 05/13/2018 03:50 AM, Stefan Weil wrote:
> This fixes cross builds for the (rare) case where cross binutils
> but no native binutils are installed.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>   configure | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 

> +strings="${STRINGS-${cross_prefix}strings}"
>   strip="${STRIP-${cross_prefix}strip}"

Hmm - we have pre-existing problems in our configure file.  More on that 
below...

>   windres="${WINDRES-${cross_prefix}windres}"
>   pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
> @@ -1956,9 +1957,9 @@ int main(int argc, char *argv[]) {
>   EOF
>   
>   if compile_object ; then
> -    if strings -a $TMPO | grep -q BiGeNdIaN ; then
> +    if "$strings" -a $TMPO | grep -q BiGeNdIaN ; then

I'd much prefer this to be:

if $strings -a $TMPO | grep...

That's because if I have something like this in my environment:

STRINGS='/path/to/strings -a'

it will only work if you allow word splitting on my variable.

...having said that, here's the pre-existing problem with user 
replacement tools that contain whitespace:

if test "$strip_opt" = "yes" ; then
   echo "STRIP=${strip}" >> $config_host_mak
fi

we are improperly quoting the whitespace when populating 
$config_host_mak, and therefore could end up attempting to execute the 
user's first argument as a standalone command, rather than the intended 
result of assigning a two-word command to a single variable.
Peter Maydell May 15, 2018, 3:04 p.m. UTC | #4
On 15 May 2018 at 16:00, Eric Blake <eblake@redhat.com> wrote:
> On 05/13/2018 03:50 AM, Stefan Weil wrote:
>>
>> This fixes cross builds for the (rare) case where cross binutils
>> but no native binutils are installed.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>   configure | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>
>> +strings="${STRINGS-${cross_prefix}strings}"
>>   strip="${STRIP-${cross_prefix}strip}"
>
>
> Hmm - we have pre-existing problems in our configure file.  More on that
> below...
>
>>   windres="${WINDRES-${cross_prefix}windres}"
>>   pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>> @@ -1956,9 +1957,9 @@ int main(int argc, char *argv[]) {
>>   EOF
>>     if compile_object ; then
>> -    if strings -a $TMPO | grep -q BiGeNdIaN ; then
>> +    if "$strings" -a $TMPO | grep -q BiGeNdIaN ; then
>
>
> I'd much prefer this to be:
>
> if $strings -a $TMPO | grep...
>
> That's because if I have something like this in my environment:
>
> STRINGS='/path/to/strings -a'
>
> it will only work if you allow word splitting on my variable.

Conversely, if I have STRINGS='/path with spaces/to/strings'
in my environment it will only work if you don't do word
splitting on it :-)

thanks
-- PMM
Eric Blake May 15, 2018, 3:52 p.m. UTC | #5
On 05/15/2018 10:04 AM, Peter Maydell wrote:

>> I'd much prefer this to be:
>>
>> if $strings -a $TMPO | grep...
>>
>> That's because if I have something like this in my environment:
>>
>> STRINGS='/path/to/strings -a'
>>
>> it will only work if you allow word splitting on my variable.
> 
> Conversely, if I have STRINGS='/path with spaces/to/strings'
> in my environment it will only work if you don't do word
> splitting on it :-)

Make convention has long been that /path with spaces/ is unacceptable 
for any of the typical tool replacements, and that you always perform 
unquoted (and thus word splitting) of a tool name.  My most common 
example is EDITOR='emacs -nw'.  You can always add a symlink to a tool 
from a path without spaces, if you want to provide a tool override to 
something that normally lives somewhere with spaces.
diff mbox

Patch

diff --git a/configure b/configure
index 24c411e346..4f6ace1ed4 100755
--- a/configure
+++ b/configure
@@ -517,6 +517,7 @@  objcopy="${OBJCOPY-${cross_prefix}objcopy}"
 ld="${LD-${cross_prefix}ld}"
 ranlib="${RANLIB-${cross_prefix}ranlib}"
 nm="${NM-${cross_prefix}nm}"
+strings="${STRINGS-${cross_prefix}strings}"
 strip="${STRIP-${cross_prefix}strip}"
 windres="${WINDRES-${cross_prefix}windres}"
 pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
@@ -1956,9 +1957,9 @@  int main(int argc, char *argv[]) {
 EOF
 
 if compile_object ; then
-    if strings -a $TMPO | grep -q BiGeNdIaN ; then
+    if "$strings" -a $TMPO | grep -q BiGeNdIaN ; then
         bigendian="yes"
-    elif strings -a $TMPO | grep -q LiTtLeEnDiAn ; then
+    elif "$strings" -a $TMPO | grep -q LiTtLeEnDiAn ; then
         bigendian="no"
     else
         echo big/little test failed