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

[Ralf Wildenhues]

Hi Eric,

* Eric Blake wrote on Mon, Nov 17, 2008 at 12:49:25AM CET:
> 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.

Yeah, I wasn't.  In some places it felt awkward or isn't portable, in
some I simply forgot.

> >      $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.

What about $progname?  Ah, it should be a basename.  Well, our current
basename functions don't compute the basename of files with backslash
as directory separator.

> > -    $ECHO "host: $host"
> > +    echo "host: $host"
> 
> Are we guaranteed that $host never contains \?

Yes, looking at config.guess and config.sub, I think so.  I'd anyway
consider it a bug in those scripts if they output something not in the
portable file name set of characters: it should be possible to use
  mkdir build/`config.guess`

> > -   $ECHO "export $shlibpath_var"
> > +   echo "export $shlibpath_var"
> 
> Likewise, for shlibpath_var.

Yes, it contains the name of a shell variable.

> > -     $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.

I generally checked these.

> > @@ -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.

Exactly.

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

Well, $SED is not initialized in the sub script, so yes, I meant that.
I guess initializing it would be cleaner, though.

> >             $ECHO "*** because the file extensions .$libext of this 
> > argument makes me believe"

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

\ would be unlikely, I guess.

> > -     output_la=`$ECHO "X$output" | $Xsed -e "$basename"`
> > +     output_la=`$ECHO "$output" | $SED "$basename"`
> 
> Wouldn't func_basename be better?

Yes, definitely; I intended to fix some of these instances in a separate
patch.

> > -[$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.

Ah, thanks.

> > -#    <var>='`$ECHO "X$<var>" | $Xsed -e "$delay_single_quote_subst"`'
> > +#    <var>='`$ECHO "$<var>" | $$SED "$delay_single_quote_subst"`'
> 
> Typo in this comment?

Looks like it.  Thanks!

> >        # 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?

Well.  This code is meant to find out the maximum command line length
limit.  It is supposed to fork and exec.  Thinking about it, I wonder.
It will likely fork with most shells (but not all), but chances are not
that high that it will exec.  Means the test looks broken anyway.  :-/

> > -                     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?

No, and I'm glad you ask about this.  The point here is that there is
more than one argument following, and I didn't see an easy way to escape
them properly, so I figured take the less likely of the bugs, as
RM="$ECHO rm -f" is quite certain to do the wrong thing.  Suggestions?

Thanks!
Ralf