libtool-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 1/7] syntax-check: fix violations and implement sc_useless_quotes


From: Gary V. Vaughan
Subject: [PATCH 1/7] syntax-check: fix violations and implement sc_useless_quotes_in_case.
Date: Mon, 21 Nov 2011 21:47:25 +0700

More syntax-check inspired goodness to make our shell code more
readable and maintainable.  There's nothing controversial in here,
except perhaps a seemingly large amount of code churn for a
relatively small gain in each case, however the code definitely
*does* need all the help it can get to be maintainable without
severe hair loss, and even a journey of 1000 miles starts with a
single step, so I'll push the whole series in 72 hours or so as
usual.

By the end of this series, we've made some good progress in
unifying the style of new and old shell code alike, and since it's
all hooked into syntax-check, it'll take a concerted effort to
regress the aspects of style I've tackled, since make distcheck
will now fail if new violations are added in future. But, making
some of the older stuff not look like spaghetti is a much much
bigger task than these few patches are able to achieve.

Feedback welcome.

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.
* cfg.mk (sc_useless_quotes_in_case): New syntax-check rule to
ensure we don't reintroduce useless quoted case expressions.
* build-aux/ltmain.m4sh, m4/libtool.m4, tests/bindir.at,
tests/darwin.at, tests/defs.m4sh, tests/demo-hardcode.test,
tests/demo-nopic.test, tests/link-2.test, tests/quote.test,
tests/sysroot.at: Remove spurious quotes.

Signed-off-by: Gary V. Vaughan <address@hidden>
---
 build-aux/ltmain.m4sh    |   12 ++++++------
 cfg.mk                   |    8 ++++++++
 m4/libtool.m4            |    2 +-
 tests/bindir.at          |    4 ++--
 tests/darwin.at          |    2 +-
 tests/defs.m4sh          |    2 +-
 tests/demo-hardcode.test |    4 ++--
 tests/demo-nopic.test    |    6 +++---
 tests/link-2.test        |    2 +-
 tests/quote.test         |    8 ++++----
 tests/sysroot.at         |    2 +-
 11 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/build-aux/ltmain.m4sh b/build-aux/ltmain.m4sh
index 5e8fb25..ee93a21 100644
--- a/build-aux/ltmain.m4sh
+++ b/build-aux/ltmain.m4sh
@@ -461,7 +461,7 @@ func_lalib_unsafe_p ()
        for lalib_p_l in 1 2 3 4
        do
            read lalib_p_line
-           case "$lalib_p_line" in
+           case $lalib_p_line in
                \#\ Generated\ by\ *$PACKAGE* ) lalib_p=yes; break;;
            esac
        done
@@ -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"*)
     func_stripname "$lt_sysroot" '' "$1"
     func_replace_sysroot_result="=$func_stripname_result"
@@ -3625,7 +3625,7 @@ main (int argc, char *argv[])
       if (STREQ (argv[i], dumpscript_opt))
        {
 EOF
-           case "$host" in
+           case $host in
              *mingw* | *cygwin* )
                # make stdout use "unix" line endings
                echo "          setmode(1,_O_BINARY);"
@@ -5796,7 +5796,7 @@ func_mode_link ()
          if test -z "$libdir" && test "$linkmode" = prog; then
            func_fatal_error "only libraries may -dlpreopen a convenience 
library: \`$lib'"
          fi
-         case "$host" in
+         case $host in
            # special handling for platforms with PE-DLLs.
            *cygwin* | *mingw* | *cegcc* )
              # Linker will automatically link against shared library if both
@@ -5896,7 +5896,7 @@ func_mode_link ()
            # We need to hardcode the library path
            if test -n "$shlibpath_var" && test -z "$avoidtemprpath" ; then
              # Make sure the rpath contains only unique directories.
-             case "$temp_rpath:" in
+             case $temp_rpath: in
              *"$absdir:"*) ;;
              *) func_append temp_rpath "$absdir:" ;;
              esac
@@ -8765,7 +8765,7 @@ func_mode_uninstall ()
          done
          test -n "$old_library" && func_append rmfiles " $odir/$old_library"
 
-         case "$opt_mode" in
+         case $opt_mode in
          clean)
            case " $library_names " in
            *" $dlname "*) ;;
diff --git a/cfg.mk b/cfg.mk
index 6a1748c..4bc32a7 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -60,6 +60,14 @@ sc_trailing_blank-non-rfc3676:
        halt='found trailing blank(s)'                                  \
          $(_sc_search_regexp)
 
+# Avoid useless quotes around case arguments such as:
+#   case "$foo" in ...
+exclude_file_name_regexp--sc_useless_quotes_in_case = ^cfg.mk$$
+sc_useless_quotes_in_case:
+       @prohibit='case "[^      "]*"[   ][      ]*in'                  \
+       halt='found spurious quotes around case argument'               \
+         $(_sc_search_regexp)
+
 # 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/m4/libtool.m4 b/m4/libtool.m4
index a9e20cf..ec00e81 100644
--- a/m4/libtool.m4
+++ b/m4/libtool.m4
@@ -1198,7 +1198,7 @@ func_echo_all ()
     $ECHO "$*"
 }
 
-case "$ECHO" in
+case $ECHO in
   printf*) AC_MSG_RESULT([printf]) ;;
   print*) AC_MSG_RESULT([print -r]) ;;
   *) AC_MSG_RESULT([cat]) ;;
diff --git a/tests/bindir.at b/tests/bindir.at
index 4e2fecc..a3684a9 100644
--- a/tests/bindir.at
+++ b/tests/bindir.at
@@ -64,7 +64,7 @@
 AT_SETUP([bindir basic lib test])
 
 bindirneeded=:
-case "$host_os" in
+case $host_os in
   cygwin*|mingw*|cegcc*)
     ;;
   *)
@@ -173,7 +173,7 @@ AT_CLEANUP
 AT_SETUP([bindir install tests])
 
 bindirneeded=:
-case "$host_os" in
+case $host_os in
   cygwin*|mingw*|cegcc*)
     ;;
   *)
diff --git a/tests/darwin.at b/tests/darwin.at
index 6eab4cd..e2abe1a 100644
--- a/tests/darwin.at
+++ b/tests/darwin.at
@@ -25,7 +25,7 @@
 AT_BANNER([Mac OS X tests])
 AT_SETUP([darwin fat compile])
 noskip=:
-case "$host_os" in
+case $host_os in
 darwin*) ;;
 *) noskip=false ;;
 esac
diff --git a/tests/defs.m4sh b/tests/defs.m4sh
index 56f1ecc..c09657f 100644
--- a/tests/defs.m4sh
+++ b/tests/defs.m4sh
@@ -45,7 +45,7 @@ fi
 # How verbose should we be?  Default is test output in log file.
 # Setting VERBOSE=debug puts the shell in debug mode.
 debug_cmd=:
-case "$VERBOSE" in
+case $VERBOSE in
 DEBUG | debug )
   debug_cmd='set -x'
   $debug_cmd
diff --git a/tests/demo-hardcode.test b/tests/demo-hardcode.test
index 31b2e1d..0e48690 100755
--- a/tests/demo-hardcode.test
+++ b/tests/demo-hardcode.test
@@ -49,7 +49,7 @@ hardcode_libdir_flag_spec' "./libtool --config" ": fatal"
 
 echo "= Searching for hardcoded library directories in each program"
 for file in hc-*; do
-  case "$file" in
+  case $file in
   hc-direct)  expected="$hardcode_direct" ;;
   hc-libpath) expected="$hardcode_shlibpath_var" ;;
   hc-minusL)  expected="$hardcode_minus_L" ;;
@@ -96,7 +96,7 @@ for file in hc-*; do
   fi
 
   # Check the result.
-  case "$hardcoded" in
+  case $hardcoded in
   yes)
     if test $expected = yes; then
       echo "$objdir was hardcoded in \`$file', as libtool expected"
diff --git a/tests/demo-nopic.test b/tests/demo-nopic.test
index acceca6..ab7c1aa 100755
--- a/tests/demo-nopic.test
+++ b/tests/demo-nopic.test
@@ -25,7 +25,7 @@
 
 . tests/defs || exit 1
 
-case "$host" in
+case $host in
 hppa*|x86_64*|s390*)
        func_skip "$host doesn't like non-PIC shared libs"
        ;;
@@ -36,10 +36,10 @@ esac
 
 if test "$build" = "$host" && test -d "/etc/selinux"; then
        _selinux=`getenforce 2>/dev/null || echo "Disabled"`
-       case "${_selinux}" in
+       case ${_selinux} in
        *Enforcing)
                _sebool_allow_execmod=`getsebool allow_execmod 2>/dev/null`
-               case "${_sebool_allow_execmod}" in
+               case ${_sebool_allow_execmod} in
                        *off)
                                func_skip "SELinux policy disallows"
                                ;;
diff --git a/tests/link-2.test b/tests/link-2.test
index 6c0f519..9c7c8c6 100755
--- a/tests/link-2.test
+++ b/tests/link-2.test
@@ -41,7 +41,7 @@ rm -f hell.lo
 test $res -eq 0 || exit $EXIT_FAILURE
 
 echo "$linkresult"
-case "$linkresult" in
+case $linkresult in
 *".lo "*)
   func_fail "$progname: .lo files should not be linked into programs"
   ;;
diff --git a/tests/quote.test b/tests/quote.test
index 6a62dac..1b986d1 100755
--- a/tests/quote.test
+++ b/tests/quote.test
@@ -47,7 +47,7 @@ for mode in compile link install; do
   # try metacharacters in the options it needs to pass to other programs.
 
   # preargs and postargs need to go through libtool unmodified.
-  case "$mode" in
+  case $mode in
   compile)
     preargs="$CC -c"
     preflag=
@@ -83,7 +83,7 @@ for mode in compile link install; do
   # may modify them.  For example, on Cygwin, ``libtool --mode=link gcc -o
   # foo foo.o''  becomes ``gcc -o foo.exe foo.o''.
   match="${match_preflag}${flag}test "
-  case "$result" in
+  case $result in
   *"$match"*)
     $ECHO "= passed: $result"
     ;;
@@ -99,7 +99,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} "
     alt_match="${match_preflag}\"${flag}\\${mchar}test\\${mchar}\" "
-    case "$result" in
+    case $result in
     *"$match"*)
       $ECHO "= passed: $result"
       ;;
@@ -120,7 +120,7 @@ for mode in compile link install; do
     $ECHO "= trying: \"$mchar\" quoting"
     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
+    case $result in
     *"$match"*)
       $ECHO "= passed: $result"
       ;;
diff --git a/tests/sysroot.at b/tests/sysroot.at
index 2a27134..90aab6f 100644
--- a/tests/sysroot.at
+++ b/tests/sysroot.at
@@ -37,7 +37,7 @@ prefix=
 for i in crt0.o crt1.o crt2.o crti.o; do
   j=`$CC --print-file-name $i 2> /dev/null`
   test $? = 0 || continue
-  case "$j" in
+  case $j in
     $gcc_sysroot*/lib/$i)
       prefix=`echo "$j" | sed "s,^$gcc_sysroot\\(.*\\)/lib/$i\$,\\1,"`
       break ;;
-- 
1.7.7.4

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]