Hi Eric, On 21 Nov 2011, at 23:15, Eric Blake wrote: > On 11/21/2011 07:47 AM, Gary V. Vaughan wrote: >> Contrary to popular belief, Bourne shell does not resplit case >> expressions after expansion, so if there are no shell unquoted >> shell metacharacters or whitespace, the quotes are useless. >> @@ -568,7 +568,7 @@ func_resolve_sysroot () >> # store the result into func_replace_sysroot_result. >> func_replace_sysroot () >> { >> - case "$lt_sysroot:$1" in >> + case $lt_sysroot:$1 in >> ?*:"$lt_sysroot"*) > > Likewise in the pattern expression; you could further change this to: > > case $lt_sysroot:$1 in > ?*:$lt_sysroot*)
Good call, although narrowing the search down to eliminate false positives is a lot trickier in this case! I'm adding the following changeset to this series. Thanks! Contrary to popular belief, Bourne shell does not resplit case branch expressions after expansion, so if there are no unquoted shell metacharacters or whitespace, the quotes are useless. * cfg.mk (sc_useless_quotes_in_case_branch): New syntax-check rule to ensure we don't reintroduce useless quoted case branch expressions. * bootstrap, build-aux/ltmain.m4sh, tests/quote.test: Remove spurious quotes. Signed-off-by: Gary V. Vaughan <g...@gnu.org> --- bootstrap | 2 +- build-aux/ltmain.m4sh | 26 +++++++++++++------------- cfg.mk | 16 ++++++++++++++++ tests/quote.test | 8 ++++---- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/bootstrap b/bootstrap index afdde66..49685f2 100755 --- a/bootstrap +++ b/bootstrap @@ -1782,7 +1782,7 @@ func_append_u () _G_delim=`expr "$2" : '\(.\)'` case $_G_delim$_G_current_value$_G_delim in - *"$2$_G_delim"*) ;; + *$2$_G_delim*) ;; *) func_append "$@" ;; esac } diff --git a/build-aux/ltmain.m4sh b/build-aux/ltmain.m4sh index 8955838..5cd201f 100644 --- a/build-aux/ltmain.m4sh +++ b/build-aux/ltmain.m4sh @@ -569,7 +569,7 @@ func_resolve_sysroot () func_replace_sysroot () { case $lt_sysroot:$1 in - ?*:"$lt_sysroot"*) + ?*:$lt_sysroot*) func_stripname "$lt_sysroot" '' "$1" func_replace_sysroot_result='='$func_stripname_result ;; @@ -895,7 +895,7 @@ func_to_tool_file () $debug_cmd case ,$2, in - *,"$to_tool_file_cmd",*) + *,$to_tool_file_cmd,*) func_to_tool_file_result=$1 ;; *) @@ -4836,12 +4836,12 @@ func_mode_link () *-*-cygwin* | *-*-mingw* | *-*-pw32* | *-*-os2* | *-cegcc*) testbindir=`$ECHO "$dir" | $SED 's*/lib$*/bin*'` case :$dllsearchpath: in - *":$dir:"*) ;; + *:$dir:*) ;; ::) dllsearchpath=$dir;; *) func_append dllsearchpath ":$dir";; esac case :$dllsearchpath: in - *":$testbindir:"*) ;; + *:$testbindir:*) ;; ::) dllsearchpath=$testbindir;; *) func_append dllsearchpath ":$testbindir";; esac @@ -5895,7 +5895,7 @@ func_mode_link () if test -n "$shlibpath_var" && test -z "$avoidtemprpath" ; then # Make sure the rpath contains only unique directories. case $temp_rpath: in - *"$absdir:"*) ;; + *$absdir:*) ;; *) func_append temp_rpath "$absdir:" ;; esac fi @@ -6122,7 +6122,7 @@ func_mode_link () if test -n "$add_shlibpath"; then case :$compile_shlibpath: in - *":$add_shlibpath:"*) ;; + *:$add_shlibpath:*) ;; *) func_append compile_shlibpath "$add_shlibpath:" ;; esac fi @@ -6136,7 +6136,7 @@ func_mode_link () test yes != "$hardcode_minus_L" && test yes = "$hardcode_shlibpath_var"; then case :$finalize_shlibpath: in - *":$libdir:"*) ;; + *:$libdir:*) ;; *) func_append finalize_shlibpath "$libdir:" ;; esac fi @@ -6156,7 +6156,7 @@ func_mode_link () add=-l$name elif test yes = "$hardcode_shlibpath_var"; then case :$finalize_shlibpath: in - *":$libdir:"*) ;; + *:$libdir:*) ;; *) func_append finalize_shlibpath "$libdir:" ;; esac add=-l$name @@ -7307,7 +7307,7 @@ EOF else # Just accumulate the unique libdirs. case $hardcode_libdir_separator$hardcode_libdirs$hardcode_libdir_separator in - *"$hardcode_libdir_separator$libdir$hardcode_libdir_separator"*) + *$hardcode_libdir_separator$libdir$hardcode_libdir_separator*) ;; *) func_append hardcode_libdirs "$hardcode_libdir_separator$libdir" @@ -8034,7 +8034,7 @@ EOF else # Just accumulate the unique libdirs. case $hardcode_libdir_separator$hardcode_libdirs$hardcode_libdir_separator in - *"$hardcode_libdir_separator$libdir$hardcode_libdir_separator"*) + *$hardcode_libdir_separator$libdir$hardcode_libdir_separator*) ;; *) func_append hardcode_libdirs "$hardcode_libdir_separator$libdir" @@ -8055,12 +8055,12 @@ EOF *-*-cygwin* | *-*-mingw* | *-*-pw32* | *-*-os2* | *-cegcc*) testbindir=`$ECHO "$libdir" | $SED -e 's*/lib$*/bin*'` case :$dllsearchpath: in - *":$libdir:"*) ;; + *:$libdir:*) ;; ::) dllsearchpath=$libdir;; *) func_append dllsearchpath ":$libdir";; esac case :$dllsearchpath: in - *":$testbindir:"*) ;; + *:$testbindir:*) ;; ::) dllsearchpath=$testbindir;; *) func_append dllsearchpath ":$testbindir";; esac @@ -8085,7 +8085,7 @@ EOF else # Just accumulate the unique libdirs. case $hardcode_libdir_separator$hardcode_libdirs$hardcode_libdir_separator in - *"$hardcode_libdir_separator$libdir$hardcode_libdir_separator"*) + *$hardcode_libdir_separator$libdir$hardcode_libdir_separator*) ;; *) func_append hardcode_libdirs "$hardcode_libdir_separator$libdir" diff --git a/cfg.mk b/cfg.mk index 1967309..80d299e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -231,6 +231,22 @@ sc_useless_quotes_in_case: halt='found spurious quotes around case argument' \ $(_sc_search_regexp) +# Avoid useless quotes around case branch expressions such as: +# *"$foo"*) ... +exclude_file_name_regexp--sc_useless_quotes_in_case_branch = (^cfg.mk|\.[ch]|\.diff|\.texi)$$ +sc_useless_quotes_in_case_branch: + @grep -n . $$($(VC_LIST_EXCEPT)) \ + |sed -n '/[ ]case[ ]/,/[ ]esac/{ \ + s|\([1-9][0-9]*:\).*[ ]case[ ]|\1|; \ + s|[ ]esac.*$$||; \ + p; \ + }' \ + |grep -E ':[0-9]+:[^()"]*"[^ "()]*[^ "\\]"[^"()]*\)'\ + && { \ + msg='found spurious quotes around case branch expression' \ + $(_sc_say_and_exit) \ + } || : + # List syntax-check exempted files. exclude_file_name_regexp--sc_bindtextdomain = ^tests/.*demo[0-9]*/.*\.c$$ exclude_file_name_regexp--sc_error_message_uppercase = \ diff --git a/tests/quote.test b/tests/quote.test index 7723be7..2b5734c 100755 --- a/tests/quote.test +++ b/tests/quote.test @@ -84,7 +84,7 @@ for mode in compile link install; do # foo foo.o'' becomes ``gcc -o foo.exe foo.o''. match="$match_preflag$flag:test " case $result in - *"$match"*) + *$match*) $ECHO "= passed: $result" ;; *) @@ -100,10 +100,10 @@ for mode in compile link install; do match="$match_preflag$flag\\$mchar:test\\$mchar " alt_match="$match_preflag\"$flag\\$mchar:test\\$mchar\" " case $result in - *"$match"*) + *$match*) $ECHO "= passed: $result" ;; - *"$alt_match"*) + *$alt_match*) $ECHO "= passed (harmless ksh bug): $result" ;; *) @@ -121,7 +121,7 @@ for mode in compile link install; do result=`$LIBTOOL -n --mode=$mode $preargs $preflag"$flag$mchar:test$mchar" $postargs` || status=$EXIT_FAILURE match="$match_preflag\"$flag$mchar:test$mchar\" " case $result in - *"$match"*) + *$match*) $ECHO "= passed: $result" ;; *) -- 1.7.7.4 Cheers, -- Gary V. Vaughan (gary AT gnu DOT org)