Re: [PATCH 8/7] syntax-check: fix violations and implement sc_useless_quotes_in_case_branch.

2011-11-22 Thread Gary V. Vaughan 
Hi Eric,

On 22 Nov 2011, at 12:09, Gary V. Vaughan wrote:
 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!

Actually, no, I take it back.  I won't apply that patch since it causes tests
to fail for at least dash, bash and ksh.

My reading of the opengroup specification agrees with us both that no 
resplitting
should be done in the branch expressions of a case, but there is definitely
something odd going on that I don't really understand:

  $ cat case.test
  for match in '\$\$' '\$'; do
  case $match in *$match*)   echo good ;; *) echo bad ;; esac
  case $match in *$match*) echo good ;; *) echo bad ;; esac
  done

  solaris10 /bin/sh$ sh case.test
  good
  good
  good
  good

  solaris10 xpg4$ /usr/xpg4/bin/sh case.test
  good
  good
  good
  good

  hpux10 /bin/sh$ sh case.test
  good
  good
  good
  good

  aix6 /bin/sh$ sh case.test
  good
  good
  good
  good

  zsh-4.3.10$ zsh case.test
  good
  good
  good
  good

  zsh-4.3.11$ /usr/local/Cellar/zsh-4.3.11/bin/zsh case.test
  good
  good
  good
  good

  zsh-4.3.12$ /usr/local/bin/zsh case.test
  good
  good
  good
  good

  dash-0.5.7$ dash case.test
  bad
  good
  good
  good

  bash-3.2.48$ bash case.test
  bad
  good
  good
  good

  bash-4.1.5$ /usr/local/Cellar/bash-4.1.5/bin/bash case.test
  bad
  good
  good
  good

  bash-4.2.10$ /usr/local/bin/bash case.test
  bad
  good
  good
  good

  ksh-93s+$ ksh case.test
  bad
  good
  good
  good

Confusing! It seems any backslash escaped character will do, same
results with '\S\S' and '\S' as with the dollars above.  Yet, with
the backslashes removed, all the above print 4 'good's.

If it weren't for the fact that ksh, bash and dash all independently
behave the same way, I'd have called it a bug...  any idea?

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

Re: [PATCH 8/7] syntax-check: fix violations and implement sc_useless_quotes_in_case_branch.

2011-11-22 Thread Eric Blake 
On 11/22/2011 01:21 AM, Gary V. Vaughan wrote:
 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!
 
 Actually, no, I take it back.  I won't apply that patch since it causes tests
 to fail for at least dash, bash and ksh.
 
 My reading of the opengroup specification agrees with us both that no 
 resplitting
 should be done in the branch expressions of a case, but there is definitely
 something odd going on that I don't really understand:
 

 
 Confusing! It seems any backslash escaped character will do, same
 results with '\S\S' and '\S' as with the dollars above.  Yet, with
 the backslashes removed, all the above print 4 'good's.
 
 If it weren't for the fact that ksh, bash and dash all independently
 behave the same way, I'd have called it a bug...  any idea?

Oh, I think I understand the issue:

$ a=\*
$ case b in $a) echo one;; *) echo two;; esac
one
$ case b in $a) echo one;; *) echo two;; esac
two

POSIX states:

each pattern that labels a compound-list shall be subjected to tilde
expansion, parameter expansion, command substitution, and arithmetic
expansion, and the result of these expansions shall be compared against
the expansion of word, according to the rules described in Section 2.13
(on page 2332) (which also describes the effect of quoting parts of the
pattern).

which in turn states:

If any character (ordinary, shell special, or pattern special) is
quoted, that pattern shall match the character itself. The shell special
characters always require quoting.

So, variable expansion is performed _prior_ to passing the pattern to
fnmatch(), but quoting can occur either by \ or by .  In the case
where $a is unquoted, we are passing fnmatch the string *, but where
$a was quoted, the shell is correctly treating * as a literal and
passing fnmatch the string \\*.

Bash, ksh, and dash are correct; mksh and Solaris 10 (among other
shells) are buggy.

And I stand corrected - variable expansion in case labels _must_ be
quoted if you want to literally match all characters that occur in that
variable expansion.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[PATCH 8/7] syntax-check: fix violations and implement sc_useless_quotes_in_case_branch.

2011-11-21 Thread Gary V. Vaughan 
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
-