On 22 Nov 2011, at 02:59, Stefano Lattarini wrote:
> Hi Gary.  Again, few quick nits here, probably incomplete.

Hi Stefano,

Thanks for taking a look again.

> On Monday 21 November 2011, Gary V wrote:
>> * cfg.mk (sc_prohibit_bare_basename, sc_prohibit_basename_with_sed):
>> Make sure not to go back to using occasional `|$basename' or
>> `|$dirname' syntax.
>> * build-aux/general.m4sh, build-aux/git-hooks/commit-msg,
>> build-aux/ltmain.m4sh, build-aux/options-parser,
>> tests/fcdemo-conf.test, tests/fcdemo-shared.test,
>> tests/fcdemo-static.test, tests/libtoolize.at: Fix violations.
>> 
>> Signed-off-by: Gary V. Vaughan <g...@gnu.org>
>> 
>> diff --git a/build-aux/general.m4sh b/build-aux/general.m4sh
>> index 1f44535..790f4e0 100644
>> --- a/build-aux/general.m4sh
>> +++ b/build-aux/general.m4sh
>> @@ -70,15 +70,15 @@ lt_nl='
>> '
>> IFS="         $lt_nl"
>> 
>> -dirname='s,/[^/]*$,,'
>> -basename='s,^.*/,,'
>> +dirname='s|/[^/]*$||'
>> +basename='s|^.*/||'
>> 
> What's the point of this change?  If it's only stylistic, shouldn't it
> be done in a separate patch?

That leaked out of the prohibit_sed_s_comma patch later in the series, so
I've moved it back to the correct patch.  Thanks.

>> # func_dirname file append nondir_replacement
>> # Compute the dirname of FILE.  If nonempty, add APPEND to the result,
>> # otherwise set result to NONDIR_REPLACEMENT.
>> func_dirname ()
>> {
>> -    func_dirname_result=`$ECHO "${1}" | $SED "$dirname"`
>> +    func_dirname_result=`$ECHO "${1}" |$SED "$dirname"`
>> 
> Ditto, and for other similar changes as well.

Not sure how that happened, maybe cut and pasting between various definitions.
Also fixed.


>> diff --git a/build-aux/ltmain.m4sh b/build-aux/ltmain.m4sh
>> index 02ff034..b367ddd 100644
>> --- a/build-aux/ltmain.m4sh
>> +++ b/build-aux/ltmain.m4sh
>> @@ -3042,7 +3042,7 @@ func_extract_archives ()
>>            $RM 
>> "unfat-$$/${darwin_base_archive}-${darwin_arch}/${darwin_base_archive}"
>>          done # $darwin_arches
>>             ## Okay now we've a bunch of thin objects, gotta fatten them up 
>> :)
>> -        darwin_filelist=`find unfat-$$ -type f ... -print | $SED -e 
>> "$basename" | sort -u`
>> +        darwin_filelist=`find unfat-$$ -type f ... -print | $SED $basename 
>> | sort -u`
>> 
> Why this quote removal?  It seems gratuitous -- even dangerous,
> since `$basename' contains shell wildcards (`*' at least).

Yikes!  Well that's embarrassing.  Nicely caught, thanks.

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)

Reply via email to