Hello,
  sorry for the delay.

On Sun, May 01, 2005 at 10:50:01PM -0700, Paul Eggert wrote:
> I don't see any problem in particular, but have you read the caveats about
> expr in the Autoconf manual?

I reviewed my patch from that perspective.  I don't use the exit code of
expr and all regexps used there should always match.
So the only potential problem is that the matched substring could be longer
than 120 chars.  There are two suspicious places:

1) $ac_top_builddir might be longer than 120 chars.

This would happen if the depth of the source tree is > 40.  Insane.

2) X$ac_file in _AC_CONFIG_SPLIT could be longer than 120 chars.

This would happen if AC_CONFIG_FOOS contained a ``tag'' which contained
a colon and which is longer than 120 chars.  Ie. something like
        Makefile:boiler/top.mk:boiler/bot.mk
but the whole string specifying the file and the templates would have to
be at least 120 chars long.
If the ``tag'' doesn't contain `:', there is no problem.
Again, I don't think this is a real limitation.

Please note that even if the source tree contains relative paths longer
then 120 chars, that doesn't mean that the package wouldn't have problems
on platforms with short-sighted expr.

To sum up, I think my patch is clean and can be commited.
(I ctreated a slight variation of the patch, please find it attached to
this post.)

Paul, OK to commit?

Stepan Kasal
2005-05-13  Stepan Kasal  <[EMAIL PROTECTED]>

        * lib/autoconf/general.m4 (_AC_INIT_HELP): Merge several smaller
          ``cat <<_ACEOF'' commands to one.
        (_AC_CANONICAL_SPLIT): Use expr, not ``echo|sed.''
        * lib/autoconf/status.m4: On various places, use expr instead of
          ``echo|sed.''
        (_AC_CONFIG_SPLIT, _AC_CONFIG_SPLIT_SOURCE_DEST):
        (_AC_CONFIG_SPLIT_FILE_IN): New macros, to factor out common code.
        * lib/autotest/general.m4 (AT_INIT): Use expr to get the numbers from
          a range.
        * tests/local.at (AT_CHECK_SHELL_SYNTAX): Use awk to search for
          the wrong patterns between ``case'' and ``esac.''  The previous
          code had false positives.

Index: lib/autoconf/general.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autoconf/general.m4,v
retrieving revision 1.854
diff -u -r1.854 general.m4
--- lib/autoconf/general.m4     13 May 2005 09:17:15 -0000      1.854
+++ lib/autoconf/general.m4     13 May 2005 09:35:28 -0000
@@ -1004,9 +1004,6 @@
   -n, --no-create         do not create output files
       --srcdir=DIR        find the sources in DIR [configure dir or \`..']
 
-_ACEOF
-
-  cat <<_ACEOF
 Installation directories:
   --prefix=PREFIX         install architecture-independent files in PREFIX
                          [$ac_default_prefix]
@@ -1040,9 +1037,7 @@
   --dvidir=DIR           dvi documentation [DOCDIR]
   --pdfdir=DIR           pdf documentation [DOCDIR]
   --psdir=DIR            ps documentation [DOCDIR]
-_ACEOF
-
-  cat <<\_ACEOF]
+]dnl
 m4_divert_pop([HELP_BEGIN])dnl
 dnl The order of the diversions here is
 dnl - HELP_BEGIN
@@ -1685,12 +1680,9 @@
 m4_define([_AC_CANONICAL_SPLIT],
 [AC_SUBST([$1],       [$ac_cv_$1])dnl
 dnl FIXME: AC_SUBST([$1_alias],  [$ac_cv_$1_alias])dnl
-AC_SUBST([$1_cpu],
-        [`echo $ac_cv_$1 | sed 's/^\([[^-]]*\)-\([[^-]]*\)-\(.*\)$/\1/'`])dnl
-AC_SUBST([$1_vendor],
-        [`echo $ac_cv_$1 | sed 's/^\([[^-]]*\)-\([[^-]]*\)-\(.*\)$/\2/'`])dnl
-AC_SUBST([$1_os],
-        [`echo $ac_cv_$1 | sed 's/^\([[^-]]*\)-\([[^-]]*\)-\(.*\)$/\3/'`])dnl
+AC_SUBST([$1_cpu],     [`expr "X$ac_cv_$1" : ['X\([^-]*\)']`])dnl
+AC_SUBST([$1_vendor],  [`expr "X$ac_cv_$1" : ['X[^-]*-\([^-]*\)']`])dnl
+AC_SUBST([$1_os],      [`expr "X$ac_cv_$1" : ['X[^-]*-[^-]*-\(.*\)']`])dnl
 ])# _AC_CANONICAL_SPLIT
 
 
Index: lib/autoconf/status.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autoconf/status.m4,v
retrieving revision 1.54
diff -u -r1.54 status.m4
--- lib/autoconf/status.m4      12 May 2005 23:46:43 -0000      1.54
+++ lib/autoconf/status.m4      13 May 2005 09:35:28 -0000
@@ -153,7 +153,7 @@
     if test -z "$ac_top_builddir"; then
        ac_top_srcdir=.
     else
-       ac_top_srcdir=`echo $ac_top_builddir | sed 's,/$,,'`
+       ac_top_srcdir=`expr $ac_top_builddir : ['\(.*[^/]\)']`
     fi
     ac_abs_top_srcdir=$ac_pwd ;;
   [[\\/]]* | ?:[[\\/]]* )  # Absolute name.
@@ -243,6 +243,44 @@
 ])
 
 
+# _AC_CONFIG_SPLIT(LIST, DEST, SOURCE)
+# ------------------------------------
+#
+# Shell variable LIST must contain at least two file names, separated by
+# colon.  The first component goes to DEST, the rest to SOURCE.
+# We compute SOURCE first, so LIST and DEST can be the same variable.
+#
+m4_define([_AC_CONFIG_SPLIT],
+[      $3=`expr "X$$1" : ['X[^:]*:\(.*\)']`
+       $2=`expr "X$$1" : ['X\([^:]*\)']`[]dnl
+])
+
+# _AC_CONFIG_SPLIT_SOURCE_DEST
+# ----------------------------
+#
+# Used in CONFIG_COMMANDS and CONFIG_LINKS sections.
+#
+m4_define([_AC_CONFIG_SPLIT_SOURCE_DEST],
+[case $ac_file in
+  *:*) _AC_CONFIG_SPLIT(ac_file, ac_dest, ac_source) ;;
+  *)   ac_dest=$ac_file ac_source=$ac_file ;;
+esac[]dnl
+])
+
+# _AC_CONFIG_SPLIT_FILE_IN
+# ------------------------
+#
+# Used in CONFIG_HEADERS and CONFIG_FILES sections.
+#
+m4_define([_AC_CONFIG_SPLIT_FILE_IN],
+[case $ac_file in
+  *:*) _AC_CONFIG_SPLIT(ac_file, ac_file, ac_file_in) ;;
+  - )  ac_file_in=- ;;
+  * )   ac_file_in=$ac_file.in ;;
+esac[]dnl
+])
+
+
 
 ## ------------------------ ##
 ## Configuration commands.  ##
@@ -363,8 +401,7 @@
 # CONFIG_COMMANDS section.
 #
 for ac_file in : $CONFIG_COMMANDS; do test "x$ac_file" = x: && continue
-  ac_dest=`echo "$ac_file" | sed 's,:.*,,'`
-  ac_source=`echo "$ac_file" | sed 's,[[^:]]*:,,'`
+  _AC_CONFIG_SPLIT_SOURCE_DEST
   ac_dir=`AS_DIRNAME(["$ac_dest"])`
   AS_MKDIR_P(["$ac_dir"])
   _AC_SRCDIRS(["$ac_dir"])
@@ -483,7 +520,7 @@
 [cat >>$CONFIG_STATUS <<\_ACEOF
 
 #
-# CONFIG_HEADER section.
+# CONFIG_HEADERS section.
 #
 
 # These sed commands are passed to sed as "A NAME B NAME C VALUE D", where
@@ -505,13 +542,9 @@
   # Support "outfile[:infile[:infile...]]", defaulting infile="outfile.in".
   case $ac_file in
   - | *:- | *:-:* ) # input from stdin
-       cat >"$tmp/stdin"
-       ac_file_in=`echo "$ac_file" | sed 's,[[^:]]*:,,'`
-       ac_file=`echo "$ac_file" | sed 's,:.*,,'` ;;
-  *:* ) ac_file_in=`echo "$ac_file" | sed 's,[[^:]]*:,,'`
-       ac_file=`echo "$ac_file" | sed 's,:.*,,'` ;;
-  * )   ac_file_in=$ac_file.in ;;
+       cat >"$tmp/stdin" ;;
   esac
+  _AC_CONFIG_SPLIT_FILE_IN
 
   test x"$ac_file" != x- && AC_MSG_NOTICE([creating $ac_file])
 
@@ -727,7 +760,7 @@
 # interest in creating config links with literal values, no special
 # mechanism is implemented to handle them.
 #
-# _AC_LINK_CNT is used to be robust to multiple calls.
+# _AC_LINK_FILES_CNT is used to be robust to multiple calls.
 AU_DEFUN([AC_LINK_FILES],
 [m4_if($#, 2, ,
        [m4_fatal([$0: incorrect number of arguments])])dnl
@@ -766,8 +799,7 @@
 dnl with empty parameters (as in gettext.m4), then we obtain here
 dnl `:', which we want to skip.  So let's keep a single exception: `:'.
 for ac_file in : $CONFIG_LINKS; do test "x$ac_file" = x: && continue
-  ac_dest=`echo "$ac_file" | sed 's,:.*,,'`
-  ac_source=`echo "$ac_file" | sed 's,[[^:]]*:,,'`
+  _AC_CONFIG_SPLIT_SOURCE_DEST
 
   AC_MSG_NOTICE([linking $srcdir/$ac_source to $ac_dest])
 
@@ -932,13 +964,9 @@
   # Support "outfile[:infile[:infile...]]", defaulting infile="outfile.in".
   case $ac_file in
   - | *:- | *:-:* ) # input from stdin
-       cat >"$tmp/stdin"
-       ac_file_in=`echo "$ac_file" | sed 's,[[^:]]*:,,'`
-       ac_file=`echo "$ac_file" | sed 's,:.*,,'` ;;
-  *:* ) ac_file_in=`echo "$ac_file" | sed 's,[[^:]]*:,,'`
-       ac_file=`echo "$ac_file" | sed 's,:.*,,'` ;;
-  * )   ac_file_in=$ac_file.in ;;
+       cat >"$tmp/stdin" ;;
   esac
+  _AC_CONFIG_SPLIT_FILE_IN
 
   # Compute @srcdir@, @top_srcdir@, and @INSTALL@ for subdirectories.
   ac_dir=`AS_DIRNAME(["$ac_file"])`
@@ -1405,8 +1433,8 @@
 do
   case $[1] in
   --*=*)
-    ac_option=`expr "x$[1]" : 'x\([[^=]]*\)='`
-    ac_optarg=`expr "x$[1]" : 'x[[^=]]*=\(.*\)'`
+    ac_option=`expr "X$[1]" : 'X\([[^=]]*\)='`
+    ac_optarg=`expr "X$[1]" : 'X[[^=]]*=\(.*\)'`
     ac_shift=:
     ;;
   -*)
Index: lib/autotest/general.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autotest/general.m4,v
retrieving revision 1.176
diff -u -r1.176 general.m4
--- lib/autotest/general.m4     29 Apr 2005 21:16:07 -0000      1.176
+++ lib/autotest/general.m4     13 May 2005 09:35:29 -0000
@@ -320,8 +320,9 @@
     [[0-9][0-9][0-9]-[0-9][0-9][0-9]] | \
     [[0-9][0-9][0-9]-[0-9][0-9][0-9][0-9]] | \
     [[0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9]] )
-       at_range_start=`echo $at_option |sed 's,-.*,,'`
-       at_range_end=`echo $at_option |sed 's,.*-,,'`
+       # No need to use "X$at_option", we know how $at_option looks like.
+       at_range_start=`expr $at_option : '\([^-]*\)'`
+       at_range_end=`expr $at_option : '[^-]*-\(.*\)'`
        # FIXME: Maybe test to make sure start <= end?
        at_range=`echo " $at_groups_all " | \
          sed -e 's,^.* '$at_range_start' ,'$at_range_start' ,' \
Index: tests/local.at
===================================================================
RCS file: /cvsroot/autoconf/autoconf/tests/local.at,v
retrieving revision 1.13
diff -u -r1.13 local.at
--- tests/local.at      5 Feb 2005 07:58:43 -0000       1.13
+++ tests/local.at      13 May 2005 09:35:29 -0000
@@ -53,7 +53,7 @@
 m4_define([AT_CHECK_SHELL_SYNTAX],
 [AS_IF([test x"$ac_cv_sh_n_works" != xno],
   [AT_CHECK([/bin/sh -n $1], 0)], [$2])
-AT_CHECK([grep '\@<:@\^.*).*;;' $1], 1)])
+AT_CHECK([awk '/^[ \t]*case/,/^[ \t]*esac/{if(/\@<:@\^.*\)/) exit(1)}' $1])])
 
 m4_define([AT_CHECK_PERL_SYNTAX],
 [AT_CHECK([autom4te_perllibdir=$abs_top_srcdir/lib $PERL -c 
$abs_top_builddir/bin/$1],

Reply via email to