Re: [PATCH 4/7] syntax-check: fix violations and implement sc_prohibit_bare_basename.
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
>>
>> 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)
Re: [PATCH 4/7] syntax-check: fix violations and implement sc_prohibit_bare_basename.
Hi Gary. Again, few quick nits here, probably incomplete.
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
>
> 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?
> # 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.
> 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).
Regards,
Stefano
[PATCH 4/7] syntax-check: fix violations and implement sc_prohibit_bare_basename.
* 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
---
build-aux/general.m4sh | 12 ++--
build-aux/git-hooks/commit-msg |4 ++--
build-aux/ltmain.m4sh |2 +-
build-aux/options-parser |6 +++---
cfg.mk | 11 +++
tests/fcdemo-conf.test |2 +-
tests/fcdemo-shared.test |2 +-
tests/fcdemo-static.test |2 +-
tests/libtoolize.at|4 ++--
9 files changed, 28 insertions(+), 17 deletions(-)
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|^.*/||'
# 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"`
if test "X$func_dirname_result" = "X${1}"; then
func_dirname_result=${3}
else
@@ -90,7 +90,7 @@ func_dirname ()
# func_basename file
func_basename ()
{
-func_basename_result=`$ECHO "${1}" | $SED "$basename"`
+func_basename_result=`$ECHO "${1}" |$SED "$basename"`
} # func_basename may be replaced by extended shell implementation
@@ -109,13 +109,13 @@ func_basename ()
func_dirname_and_basename ()
{
# Extract subdirectory from the argument.
-func_dirname_result=`$ECHO "${1}" | $SED -e "$dirname"`
+func_dirname_result=`$ECHO "${1}" |$SED "$dirname"`
if test "X$func_dirname_result" = "X${1}"; then
func_dirname_result=${3}
else
func_dirname_result=$func_dirname_result${2}
fi
-func_basename_result=`$ECHO "${1}" | $SED -e "$basename"`
+func_basename_result=`$ECHO "${1}" |$SED "$basename"`
} # func_dirname_and_basename may be replaced by extended shell implementation
diff --git a/build-aux/git-hooks/commit-msg b/build-aux/git-hooks/commit-msg
index f2e6108..bbd7751 100755
--- a/build-aux/git-hooks/commit-msg
+++ b/build-aux/git-hooks/commit-msg
@@ -6,13 +6,13 @@
: ${SED="sed"}
test set = ${ECHO+'set'} = set || ECHO='printf %s\n'
-basename="$SED -e "'s|^.*/||'
+basename='s|^.*/||'
nl='
'
progpath=$0
-progname=`$ECHO "$progpath" |$basename`
+progname=`$ECHO "$progpath" |$SED "$basename"`
log_file=$1
export log_file
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 -name \*.o -print -o -name
\*.lo -print | $SED -e "$basename" | sort -u`
+ darwin_filelist=`find unfat-$$ -type f -name \*.o -print -o -name
\*.lo -print | $SED $basename | sort -u`
darwin_file=
darwin_files=
for darwin_file in $darwin_filelist; do
diff --git a/build-aux/options-parser b/build-aux/options-parser
index 4777d76..566ae43 100644
--- a/build-aux/options-parser
+++ b/build-aux/options-parser
@@ -167,8 +167,8 @@ EXIT_SKIP=77 # $? = 77 is used to indicate a skipped
test to automake.
debug_cmd=${debug_cmd-":"}
exit_cmd=:
-dirname="$SED -e "'s|/[^/]*$||'
-basename="$SED -e "'s|^.*/||'
+dirname='s|/[^/]*$||'
+basename='s|^.*/||'
nl='
'
@@ -181,7 +181,7 @@ nl='
progpath=$0
# The name of this program.
-progname=`echo "$progpath" |$basename`
+progname=`echo "$progpath" |$SED "$basename"`
## - ##
diff --git a/cfg.mk b/cfg.mk
index 3d2e2a6..8a44419 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -75,6 +75,17 @@ sc_prohibit_Xsed_without_X:
else :; \
fi || :
+# Use a consistent dirname and basename idiom.
+sc_prohibit_bare_basename:
+ @prohibit='\|[ ]*\$$(base|dir)name' \
+ halt='use `|$$SED "$$basename"'\'' instead of `|$$basename'\' \
+ $(_sc_search_regexp)
+
+sc_prohibit_basename_with_dollar_sed:
+ @prohibit='(base|dir)name="?(\$$SED|sed)[ "]' \
+ halt='use `basename='\''s|^.*/||'\'' instead of `basename="$$SED...'
\
+ $(_sc_search_regexp)
+
# Check for using `[' instead of `test'.
exclude_file_name_regexp--sc_prohibit_bracket_as_te
