libtool-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Revenge of the $ECHO. Kill most uses of Xsed.


From: Eric Blake
Subject: Re: Revenge of the $ECHO. Kill most uses of Xsed.
Date: Sun, 16 Nov 2008 16:49:25 -0700
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17) Gecko/20080914 Thunderbird/2.0.0.17 Mnenhy/0.7.5.666

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Ralf Wildenhues on 11/16/2008 3:48 PM:
> -        my_directory_path=`$ECHO "X$my_directory_path" | $Xsed -e "$dirname"`
> +        my_directory_path=`$ECHO "$my_directory_path" | $SED -e "$dirname"`
>        done
> -      my_dir_list=`$ECHO "X$my_dir_list" | $Xsed -e 's,:*$,,'`
> +      my_dir_list=`$ECHO "$my_dir_list" | $SED 's,:*$,,'`

You generally removed the -e (which is a good move in my mind), but
weren't consistent about it.

>      $ECHO "run \`$progname --help | more' for full usage"

This can be echo rather than $ECHO, since the \` is flattened during ""
interpratation prior to the echo call.  Several similar places exist.

> -    $ECHO "host: $host"
> +    echo "host: $host"

Are we guaranteed that $host never contains \?

> -     $ECHO "export $shlibpath_var"
> +     echo "export $shlibpath_var"

Likewise, for shlibpath_var.

>  
> -       $ECHO >> "$output_objdir/$my_dlsyms" "\
> +       echo >> "$output_objdir/$my_dlsyms" "\
>  
>  /* The mapping between symbol names and symbols.  */
>  typedef struct {
> @@ -2030,7 +2030,7 @@ typedef struct {

This lacks context to see if it is safe, or if the text being appended
contains \.  Several instance of this idiom.

> @@ -2364,13 +2363,13 @@ _LTECHO_EOF'
>    fi\
>  
>    # Find the directory that this script lives in.
> -  thisdir=\`\$ECHO \"X\$file\" | \$Xsed -e 's%/[^/]*$%%'\`
> +  thisdir=\`\$ECHO \"\$file\" | $SED 's%/[^/]*$%%'\`

Similar to Paolo's patch, my comment still holds that this area of code
would be much more maintainable with an AS_ESCAPE (or a similar
m4_bpatsubst, since AS_ESCAPE is not yet a documented m4sh interface),
rather than hand-escaping the contents of here-doc's and eval's.  But that
can (should) be a separate patch.

Meanwhile, did you really mean $SED, or did you want \$SED?

>    test \"x\$thisdir\" = \"x\$file\" && thisdir=.
>  
>    # Follow symbolic links until we get to the real thisdir.
> -  file=\`ls -ld \"\$file\" | ${SED} -n 's/.*-> //p'\`
> +  file=\`ls -ld \"\$file\" | $SED -n 's/.*-> //p'\`

Oh, there was already an existing $SED, a few lines down.

>               $ECHO "*** because the file extensions .$libext of this 
> argument makes me believe"
> -             $ECHO "*** that it is just a static archive that I should not 
> use here."
> +             echo "*** that it is just a static archive that I should not 
> use here."

Is $libext ever likely to contain \, or can we use plain echo here, too?

> -       if $ECHO "X $tmp_deplibs" | $Xsed -e 's/[      ]//g' |
> -          $GREP . >/dev/null; then
> -         $ECHO
> +       case $tmp_deplibs in
> +       *[!\  \ ]*)

Slick way to shave forks!

> -       output_la=`$ECHO "X$output" | $Xsed -e "$basename"`
> +       output_la=`$ECHO "$output" | $SED "$basename"`

Wouldn't func_basename be better?

> -[$1='`$ECHO "X$][$1" | $Xsed -e "$delay_single_quote_subst"`'])
> +[$1='`$ECHO "$][$1" | $SED "$delay_single_quote_subst"`'])

Unrelated to your patch, but this does not need ][ in the middle.  M4
outputs $$1 as a literal $ followed by the first argument, without needing
separation between the quoted strings.

> -#    <var>='`$ECHO "X$<var>" | $Xsed -e "$delay_single_quote_subst"`'
> +#    <var>='`$ECHO "$<var>" | $$SED "$delay_single_quote_subst"`'

Typo in this comment?

>        # If test is not a shell built-in, we'll probably end up computing a
>        # maximum length that is only half of the actual maximum length, but
>        # we can't tell.
> -      while { test "X"`$ECHO "X$teststring$teststring" 2>/dev/null` \
> -              = "XX$teststring$teststring"; } >/dev/null 2>&1 &&
> +      while { test "X"`$ECHO "$teststring$teststring" 2>/dev/null` \
> +              = "X$teststring$teststring"; } >/dev/null 2>&1 &&

Does $teststring contain \, or can we use echo here to avoid forks in this
loop on shells where $ECHO is expensive?

> -                       RM="$ECHO $RM"
> -                       test -n "$LN_S" && LN_S="$ECHO $LN_S"
> -                       CP="$ECHO $CP"
> -                       MKDIR="$ECHO $MKDIR"
> -                       TAR="$ECHO $TAR"
> +                       RM="echo $RM"
> +                       test -n "$LN_S" && LN_S="echo $LN_S"
> +                       CP="echo $CP"
> +                       MKDIR="echo $MKDIR"
> +                       TAR="echo $TAR"

Are all of these safe, considering mingw might have \ in an absolute
pathname to these tools?

Most of the patch looked okay to me (rather mechanical), so hopefully I
didn't let my eyes glaze over and miss an obvious typo.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkkgsYUACgkQ84KuGfSFAYCj+QCeMoBHmCGqtfkMIGZTNfrIh1Kg
mIQAoL8A+MVxpJp/ktxYzDVSNmRejKBJ
=Wq1t
-----END PGP SIGNATURE-----




reply via email to

[Prev in Thread] Current Thread [Next in Thread]