[FYI] {ylwrap-refactor} ylwrap: various refactorings, few improvements

[Stefano Lattarini]

* lib/ylwrap: Few more safety measures: abort if we fail to get the
current working directory or to chdir to the temporary directory,
and avoid doing the final cleanup if we fail to chdir back to the
original working directory.
Remove various useless use of quotes (e.g., in `case' statements
or in variable assignments).
Avoid using a runtime-determined fancy name for the temporary file
where to save the post-processed yacc-generated header; this saves
a couple of forks.
(quote_for_sed, dirname_, dos_fix_yacc_filenames): New helper
functions, to reduce code inlined in the main loop of the script.
($ocwd): New variable, contains the absolute path of the working
directory where the script was started.  This will avoid us the
need to call `pwd` multiple times.
($do_exit): Do not discard anymore the stdout/stderr of the final
`rm -rf' on the temporary directory; it's better let the user know
whether the cleanup failed.
* tests/ylwrap-pwd-fail.test: New test.
* tests/ylwrap-chdir-back-fail.test: New test.
* tests/ylwrap-cleanup-fail.test: New test.
* tests/Makefile.am (TESTS): Update.
---
 ChangeLog                         |   25 ++++++++
 lib/ylwrap                        |  122 +++++++++++++++++++++----------------
 tests/Makefile.am                 |    3 +
 tests/Makefile.in                 |    3 +
 tests/ylwrap-chdir-back-fail.test |   63 +++++++++++++++++++
 tests/ylwrap-cleanup-fail.test    |   47 ++++++++++++++
 tests/ylwrap-pwd-fail.test        |   46 ++++++++++++++
 7 files changed, 257 insertions(+), 52 deletions(-)
 create mode 100755 tests/ylwrap-chdir-back-fail.test
 create mode 100755 tests/ylwrap-cleanup-fail.test
 create mode 100755 tests/ylwrap-pwd-fail.test

diff --git a/ChangeLog b/ChangeLog
index 1634f9f..31efc5c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,30 @@
 2011-05-05  Stefano Lattarini  <address@hidden>
 
+       ylwrap: various refactorings, few improvements
+       * lib/ylwrap: Few more safety measures: abort if we fail to get the
+       current working directory or to chdir to the temporary directory,
+       and avoid doing the final cleanup if we fail to chdir back to the
+       original working directory.
+       Remove various useless use of quotes (e.g., in `case' statements
+       or in variable assignments).
+       Avoid using a runtime-determined fancy name for the temporary file
+       where to save the post-processed yacc-generated header; this saves
+       a couple of forks.
+       (quote_for_sed, dirname_, dos_fix_yacc_filenames): New helper
+       functions, to reduce code inlined in the main loop of the script.
+       ($ocwd): New variable, contains the absolute path of the working
+       directory where the script was started.  This will avoid us the
+       need to call `pwd` multiple times.
+       ($do_exit): Do not discard anymore the stdout/stderr of the final
+       `rm -rf' on the temporary directory; it's better let the user know
+       whether the cleanup failed.
+       * tests/ylwrap-pwd-fail.test: New test.
+       * tests/ylwrap-chdir-back-fail.test: New test.
+       * tests/ylwrap-cleanup-fail.test: New test.
+       * tests/Makefile.am (TESTS): Update.
+
+2011-05-05  Stefano Lattarini  <address@hidden>
+
        ylwrap: make less general and more "expert"
        Related to automake bug#7648 and PR automake/491.
        Using the fact that ylwrap is called only in specific ways from
diff --git a/lib/ylwrap b/lib/ylwrap
index 9dd3b89..74e0eeb 100755
--- a/lib/ylwrap
+++ b/lib/ylwrap
@@ -1,7 +1,7 @@
 #! /bin/sh
 # ylwrap - wrapper for lex/yacc invocations.
 
-scriptversion=2011-05-05.09; # UTC
+scriptversion=2011-05-05.10; # UTC
 
 # Copyright (C) 1996, 1997, 1998, 1999, 2001, 2002, 2003, 2004, 2005,
 # 2007, 2009, 2010, 2011 Free Software Foundation, Inc.
@@ -37,6 +37,42 @@ usage_error ()
     exit 2
 }
 
+# Escape (most) sed metacharacters in the given string, so that it can
+# be used as a literal string in a sed match or substitute command.
+quote_for_sed ()
+{
+  printf '%s\n' "$1" | sed -e 's/[].[^$\\*|]/\\&/g'
+}
+
+# For system that lacks a dirname command, we simulate it with sed.
+dirname_ ()
+{
+  dirname "$1" 2>/dev/null \
+    || printf '%s\n' "$1" | sed -e 's,\([\\/]\)[^\\/]*$,\1,'
+}
+
+# Since DOS filename conventions don't allow two dots, the DOS version
+# of Bison writes out y_tab.c instead of y.tab.c and y_tab.h instead
+# of y.tab.h.  Test to see if this is the case.
+# This function might set the global variable `$y_tab_nodot' (meant for
+# internal use only), and modify the global variable `$from'.
+dos_fix_yacc_filenames ()
+{
+  test $wrapped = yacc || return 0
+  if test -z "$y_tab_nodot"; then
+    if test -f y_tab.c || test -f y_tab.h; then
+      y_tab_nodot=yes
+    else
+      y_tab_nodot=no
+    fi
+  fi
+  if test $y_tab_nodot = yes; then
+    # Handle y_tab.c and y_tab.h output by DOS
+    test $from = y.tab.c && from=y_tab.c
+    test $from = y.tab.h && from=y_tab.h
+  fi
+}
+
 test $# -gt 0 || usage_error "missing argument"
 
 case "$1" in
@@ -122,31 +158,30 @@ if test x"$1" = x"--"; then
   shift
 fi
 
-# The program to run.
 test $# -gt 0 || usage_error "missing program to run"
 prog="$1"
 shift
 
-case "$input" in
-  [\\/]* | ?:[\\/]*)
-    # Absolute path; do nothing.
-    ;;
-  *)
-    # Relative path.  Make it absolute.
-    input="`pwd`/$input"
-    ;;
+ocwd=`pwd` || exit 1
+
+# We are going to run yacc from another directory, so make relative
+# paths in $prog and $input absolute as fit.
+case $prog in
+  [\\/]* | ?:[\\/]*) ;;
+  # If $prog does not contain any component directory, it will be
+  # looked up in $PATH, so don't modify it.
+  *[\\/]*) prog=$ocwd/$prog ;;
 esac
 
-# Make any relative path in $prog absolute.
-case "$prog" in
+case $input in
   [\\/]* | ?:[\\/]*) ;;
-  *[\\/]*) prog="`pwd`/$prog" ;;
+  *) input=$ocwd/$input ;;
 esac
 
 # FIXME: add hostname here for parallel makes that run commands on
 # other machines.  But that might take us over the 14-char limit.
 dirname=ylwrap$$
-do_exit="cd '`pwd`' && rm -rf $dirname > /dev/null 2>&1;"' (exit $ret); exit 
$ret'
+do_exit="cd '$ocwd' && rm -rf $dirname; (exit \$ret); exit \$ret"
 # The only signals which are portably trappable are 1 (SIGHUP),
 # 2 (SIGINT), 13 (SIGPIPE) and 15 (SIGTERM).
 trap "ret=129; $do_exit" 1
@@ -154,8 +189,7 @@ trap "ret=130; $do_exit" 2
 trap "ret=141; $do_exit" 13
 trap "ret=143; $do_exit" 15
 mkdir $dirname || exit 1
-
-cd $dirname
+cd $dirname || exit 1
 
 case $# in
   0) "$prog" "$input" ;;
@@ -168,36 +202,21 @@ if test $ret -eq 0; then
   set X $pairlist
   shift
 
-  if test $wrapped = yacc; then
-    # Since DOS filename conventions don't allow two dots,
-    # the DOS version of Bison writes out y_tab.c instead of y.tab.c
-    # and y_tab.h instead of y.tab.h. Test to see if this is the case.
-    y_tab_nodot="no"
-    if test -f y_tab.c || test -f y_tab.h; then
-      y_tab_nodot="yes"
-    fi
-  fi
-
   # The directory holding the input.
-  input_dir=`echo "$input" | sed -e 's,\([\\/]\)[^\\/]*$,\1,'`
-  # Quote $INPUT_DIR so we can use it in a regexp.
-  # FIXME: really we should care about more than `.' and `\'.
-  input_rx=`echo "$input_dir" | sed 's,\\\\,\\\\\\\\,g;s,\\.,\\\\.,g'`
+  input_dir=`dirname_ "$input"`
+  # Quote $input_dir so we can use it in a regexp.
+  input_rx=`quote_for_sed "$input_dir"`
 
-  while test "$#" -ne 0; do
-    from="$1"
-    ofrom="$from"
-    if test $wrapped = "yacc" && test $y_tab_nodot = "yes"; then
-      # Handle y_tab.c and y_tab.h output by DOS
-      test $from = "y.tab.c" && from="y_tab.c"
-      test $from = "y.tab.h" && from="y_tab.h"
-    fi
+  while test $# -ne 0; do
+    from=$1
+    ofrom=$from
+    dos_fix_yacc_filenames
     if test -f "$from"; then
       # If $2 is an absolute path name, then just use that,
       # otherwise prepend `../'.
-      case "$2" in
-       [\\/]* | ?:[\\/]*) target="$2";;
-       *) target="../$2";;
+      case $2 in
+        [\\/]* | ?:[\\/]*) target=$2;;
+        *) target=../$2;;
       esac
 
       # We do not want to overwrite a header file if it hasn't
@@ -206,9 +225,9 @@ if test $ret -eq 0; then
       # because it is the destination of the .y.c rule in the
       # Makefile.  Divert the output of all other files to a temporary
       # file so we can compare them to existing versions.
-      if test $wrapped = yacc && test $ofrom = "y.tab.h"; then
-       realtarget="$target"
-       target="tmp-`echo $target | sed s/.*[\\/]//g`"
+      if test $wrapped = yacc && test $ofrom = y.tab.h; then
+        realtarget=$target
+        target=header.tmp
       fi
       # Edit out `#line' or `#' directives.
       #
@@ -231,11 +250,11 @@ if test $ret -eq 0; then
           -e "s,$FROM,$TARGET," "$from" >"$target" || ret=$?
 
       # Check whether header files must be updated.
-      if test $wrapped = "yacc" && test $ofrom = "y.tab.h"; then
-       if test -f "$realtarget" && cmp -s "$realtarget" "$target"; then
-         echo "$2" is unchanged
-         rm -f "$target"
-       else
+      if test $wrapped = yacc && test $ofrom = y.tab.h; then
+       if test -f "$realtarget" && cmp -s "$realtarget" "$target"; then
+         echo "$2" is unchanged
+         rm -f "$target"
+       else
           echo updating "$2"
           mv -f "$target" "$realtarget"
         fi
@@ -254,9 +273,8 @@ if test $ret -eq 0; then
   done
 fi
 
-# Remove the directory.
-cd ..
-rm -rf $dirname
+# Remove the temporary directory.
+cd "$ocwd" && rm -rf $dirname
 
 exit $ret
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0c0968b..69e75d6 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -869,11 +869,14 @@ yflags-force-override.test \
 yflags-force-conditional.test \
 yflags-var-expand.test \
 yacclex-line.test \
+ylwrap-chdir-back-fail.test \
+ylwrap-cleanup-fail.test \
 ylwrap-extra-args.test \
 ylwrap-fail.test \
 ylwrap-lex-parallel.test \
 ylwrap-output-rename.test \
 ylwrap-path-handling.test \
+ylwrap-pwd-fail.test \
 ylwrap-signals.test \
 ylwrap-yacc-dos.test \
 ylwrap-yacc-ext.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 0aa6af5..66b0ca3 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -1140,11 +1140,14 @@ yflags-force-override.test \
 yflags-force-conditional.test \
 yflags-var-expand.test \
 yacclex-line.test \
+ylwrap-chdir-back-fail.test \
+ylwrap-cleanup-fail.test \
 ylwrap-extra-args.test \
 ylwrap-fail.test \
 ylwrap-lex-parallel.test \
 ylwrap-output-rename.test \
 ylwrap-path-handling.test \
+ylwrap-pwd-fail.test \
 ylwrap-signals.test \
 ylwrap-yacc-dos.test \
 ylwrap-yacc-ext.test \
diff --git a/tests/ylwrap-chdir-back-fail.test 
b/tests/ylwrap-chdir-back-fail.test
new file mode 100755
index 0000000..6dc9483
--- /dev/null
+++ b/tests/ylwrap-chdir-back-fail.test
@@ -0,0 +1,63 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check safety measures employed by ylwrap w.r.t. `cd' failures.
+
+. ./defs || Exit 1
+
+set -e
+
+cp "$testsrcdir"/../lib/ylwrap .
+
+ocwd=`pwd`
+
+mkdir tmp
+chmod a-x tmp
+if test -x tmp || (cd tmp); then
+  skip_ "$me: cannot drop execute bit from dirs"
+fi
+
+cat > fake-yacc <<END
+#!/bin/sh
+set -e
+chmod a-x '$ocwd'/foodir
+ls -l '$ocwd'
+echo foobar > y.tab.c
+echo zardoz > y.tab.h
+END
+chmod a+x fake-yacc
+
+mkdir foodir
+cd foodir
+# ylwrap should succeed ...
+$SHELL "$ocwd"/ylwrap --yacc .y "$ocwd"/x.y "$ocwd"/x.c "$ocwd"/fake-yacc \
+  2>"$ocwd"/stderr || { cat "$ocwd"/stderr >&2; Exit 1; }
+cd "$ocwd"
+cat stderr >&2
+# ... but should also give warnings ...
+grep "cd: .*$me\.dir/foodir" stderr
+chmod u+x foodir
+# .. and not remove the temporary directory ...
+ls -l foodir/ylwrap[0-9]*
+test -d foodir/ylwrap[0-9]*
+# ... but still, ylwrap should correctly move/copy the generated C
+# source and header files into their final location.
+cat x.c
+cat x.h
+test `cat x.c` = foobar
+test `cat x.h` = zardoz
+
+:
diff --git a/tests/ylwrap-cleanup-fail.test b/tests/ylwrap-cleanup-fail.test
new file mode 100755
index 0000000..0727c80
--- /dev/null
+++ b/tests/ylwrap-cleanup-fail.test
@@ -0,0 +1,47 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that ylwrap gives warning messages when the cleanup fails.
+
+required=ro-dir
+. ./defs || Exit 1
+
+set -e
+
+cp "$testsrcdir"/../lib/ylwrap .
+
+ocwd=`pwd`
+
+cat > fake-yacc <<'END'
+#!/bin/sh
+set -e
+echo zardoz > y.tab.c
+chmod a-w .
+END
+chmod a+x fake-yacc
+
+# ylwrap should return a successful exit status ...
+$SHELL ylwrap --yacc .y foo.y foo.c ./fake-yacc 2>stderr \
+  || { cat stderr >&2; Exit 1; }
+cat stderr >&2
+# ... but also give warnings ...
+grep "rm: .*ylwrap[0-9]" stderr
+# ... but still correctly move/copy the generated C source file
+# into its final location.
+cat foo.c
+test `cat foo.c` = zardoz
+
+:
diff --git a/tests/ylwrap-pwd-fail.test b/tests/ylwrap-pwd-fail.test
new file mode 100755
index 0000000..9c45981
--- /dev/null
+++ b/tests/ylwrap-pwd-fail.test
@@ -0,0 +1,46 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check safety measures employed by ylwrap w.r.t. `pwd' failures.
+
+. ./defs || Exit 1
+
+set -e
+
+cp "$testsrcdir"/../lib/ylwrap .
+
+ocwd=`pwd`
+
+mkdir disappear
+cd disappear
+if (cd .. && rmdir disappear && test ! -d disappear); then
+  # pwd might be a shell builtin, so ensure we test it with the same
+  # shell that will be used to run ylwrap.
+  if $SHELL -c 'pwd'; then
+    skip_ "pwd suceeds in non-existing directory"
+  else
+    $SHELL "$ocwd"/ylwrap --yacc .y "$ocwd"/foo.y "$ocwd"/foo.c ':' \
+      2>"$ocwd"/stderr && { cat "$ocwd"/stderr >&2; Exit 1; }
+    cat "$ocwd"/stderr >&2
+    cd "$ocwd"
+    ls -l
+    test ! -f foo.c
+  fi
+else
+    skip_ "current working dir can't be removed"
+fi
+
+:
-- 
1.7.2.3