Message ID | 20180513085032.27773-1-sw@weilnetz.de |
---|---|
State | New |
Headers | show |
Series | configure: Use strings command from cross development tools | expand |
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 >
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~
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.
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
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 --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
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(-)