[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 4/7] syntax-check: fix violations and implement sc_prohibit_b
From: |
Gary V. Vaughan |
Subject: |
Re: [PATCH 4/7] syntax-check: fix violations and implement sc_prohibit_bare_basename. |
Date: |
Tue, 22 Nov 2011 12:35:53 +0700 |
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 <address@hidden>
>>
>> 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)