[Top][All Lists]
[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-----
Re: Revenge of the $ECHO. Kill most uses of Xsed., Paolo Bonzini, 2008/11/17
Re: Revenge of the $ECHO. Kill most uses of Xsed., Ralf Wildenhues, 2008/11/24