all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Philipp Stephani <p.stephani2@gmail.com>,
	Jean-Christophe Helary <jean.christophe.helary@gmail.com>,
	emacs-devel <emacs-devel@gnu.org>
Subject: Re: i18n/l10n summary
Date: Thu, 1 Jun 2017 16:20:55 -0700	[thread overview]
Message-ID: <b9d78749-ac21-6c2f-33cf-ef00a6ad65ac@cs.ucla.edu> (raw)
In-Reply-To: <CAArVCkQjXLyYuDq1AnyJg0V2DWrxoVRwTXOyt0=rqKBvqvRu5w@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

On 06/01/2017 01:17 AM, Philipp Stephani wrote:
>
> - Probably there's a bug lurking because the info[n] ought to be 
> indexed by specification index, not argument index. Something like 
> (format "%1$c %1$d" ?a) will probably do the wrong thing (untested).

Sorry, I'm not following. That call returns "a 97"; isn't that the 
expected result?

> - We should ban mixing explicit and implicit field numbers, like POSIX 
> printf(3) does. The gain from allowing to mix is negligible, and it 
> makes the implementation and the documentation needlessly complex.

Sounds good, and I installed the attached.

The 1st patch fixes a performance regression introduced by calling 
strtoumax. I went whole-hog and removed all calls to strtoumax, since 
they're all performance-significant, plus it makes for one less porting 
issue to worry about.

The 2nd patch fixes the documentation along the lines that you 
suggested. And on further thought, the tradition for Emacs is to 
document supported behavior and not worry about slowing Emacs down to 
check for undocumented usage (aside from preventing crashes), so with 
that in mind the 2nd patch removes the check for %0$ (which never crashes).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-performance-by-avoiding-strtoumax.patch --]
[-- Type: text/x-patch; name="0001-Improve-performance-by-avoiding-strtoumax.patch", Size: 21709 bytes --]

From b126485b8ce3046ca47caaba2f845a39604dd76f Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 1 Jun 2017 16:03:12 -0700
Subject: [PATCH 1/2] Improve performance by avoiding strtoumax

This made (string-to-number "10") 20% faster on my old desktop,
an AMD Phenom II X4 910e running Fedora 25 x86-64.
* admin/merge-gnulib (GNULIB_MODULES): Remove strtoumax.
* lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate.
* lib/strtoul.c, lib/strtoull.c, lib/strtoumax.c, m4/strtoull.m4:
* m4/strtoumax.m4: Remove.
* src/editfns.c (str2num): New function.
(styled_format): Use it instead of strtoumax.  Use ptrdiff_t
instead of uintmax_t.  Check for integer overflow.
* src/lread.c (LEAD_INT, DOT_CHAR, TRAIL_INT, E_EXP):
Move to private scope and make them enums.
(string_to_number): Compute integer value directly during
first pass instead of revisiting it with strtoumax later.
---
 admin/merge-gnulib |  2 +-
 lib/gnulib.mk.in   | 27 +------------------------
 lib/strtoul.c      | 19 ------------------
 lib/strtoull.c     | 26 ------------------------
 lib/strtoumax.c    |  2 --
 m4/gnulib-comp.m4  | 30 ---------------------------
 m4/strtoull.m4     | 24 ----------------------
 m4/strtoumax.m4    | 28 --------------------------
 src/editfns.c      | 43 ++++++++++++++++++++++++++++-----------
 src/lread.c        | 59 +++++++++++++++++++++++-------------------------------
 10 files changed, 58 insertions(+), 202 deletions(-)
 delete mode 100644 lib/strtoul.c
 delete mode 100644 lib/strtoull.c
 delete mode 100644 lib/strtoumax.c
 delete mode 100644 m4/strtoull.m4
 delete mode 100644 m4/strtoumax.m4

diff --git a/admin/merge-gnulib b/admin/merge-gnulib
index 45e4a78..e5fb0f5 100755
--- a/admin/merge-gnulib
+++ b/admin/merge-gnulib
@@ -38,7 +38,7 @@ GNULIB_MODULES=
   manywarnings memrchr mkostemp mktime
   pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat
   sig2str socklen stat-time std-gnu11 stdalign stddef stdio
-  stpcpy strftime strtoimax strtoumax symlink sys_stat
+  stpcpy strftime strtoimax symlink sys_stat
   sys_time time time_r time_rz timegm timer-time timespec-add timespec-sub
   update-copyright utimens
   vla warnings
diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
index d23c2a5..73d3043 100644
--- a/lib/gnulib.mk.in
+++ b/lib/gnulib.mk.in
@@ -21,7 +21,7 @@
 # the same distribution terms as the rest of that program.
 #
 # Generated by gnulib-tool.
-# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=close --avoid=dup --avoid=fchdir --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=open --avoid=openat-die --avoid=opendir --avoid=raise --avoid=save-cwd --avoid=select --avoid=setenv --avoid=sigprocmask --avoid=stat --avoid=stdarg --avoid=stdbool --avoid=threadlib --avoid=tzset --avoid=unsetenv --avoid=utime --avoid=utime-h --gnu-make --makefile-name=gnulib.mk.in --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt binary-io byteswap c-ctype c-strcase careadlinkat close-stream count-leading-zeros count-one-bits count-trailing-zeros crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo faccessat fcntl fcntl-h fdatasync fdopendir filemode filevercmp flexmember fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-changelog ignore-value intprops largefile lstat manywarnings memrchr mkostemp mktime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time std-gnu11 stdalign stddef stdio stpcpy strftime strtoimax strtoumax symlink sys_stat sys_time time time_r time_rz timegm timer-time timespec-add timespec-sub update-copyright utimens vla warnings
+# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=close --avoid=dup --avoid=fchdir --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=open --avoid=openat-die --avoid=opendir --avoid=raise --avoid=save-cwd --avoid=select --avoid=setenv --avoid=sigprocmask --avoid=stat --avoid=stdarg --avoid=stdbool --avoid=threadlib --avoid=tzset --avoid=unsetenv --avoid=utime --avoid=utime-h --gnu-make --makefile-name=gnulib.mk.in --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt binary-io byteswap c-ctype c-strcase careadlinkat close-stream count-leading-zeros count-one-bits count-trailing-zeros crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dtoastr dtotimespec dup2 environ execinfo faccessat fcntl fcntl-h fdatasync fdopendir filemode filevercmp flexmember fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-changelog ignore-value intprops largefile lstat manywarnings memrchr mkostemp mktime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time std-gnu11 stdalign stddef stdio stpcpy strftime strtoimax symlink sys_stat sys_time time time_r time_rz timegm timer-time timespec-add timespec-sub update-copyright utimens vla warnings
 
 
 MOSTLYCLEANFILES += core *.stackdump
@@ -905,7 +905,6 @@ gl_GNULIB_ENABLED_getdtablesize = @gl_GNULIB_ENABLED_getdtablesize@
 gl_GNULIB_ENABLED_getgroups = @gl_GNULIB_ENABLED_getgroups@
 gl_GNULIB_ENABLED_secure_getenv = @gl_GNULIB_ENABLED_secure_getenv@
 gl_GNULIB_ENABLED_strtoll = @gl_GNULIB_ENABLED_strtoll@
-gl_GNULIB_ENABLED_strtoull = @gl_GNULIB_ENABLED_strtoull@
 gl_GNULIB_ENABLED_tempname = @gl_GNULIB_ENABLED_tempname@
 gl_LIBOBJS = @gl_LIBOBJS@
 gl_LTLIBOBJS = @gl_LTLIBOBJS@
@@ -2507,30 +2506,6 @@ EXTRA_libgnu_a_SOURCES += strtol.c strtoll.c
 endif
 ## end   gnulib module strtoll
 
-## begin gnulib module strtoull
-ifeq (,$(OMIT_GNULIB_MODULE_strtoull))
-
-ifneq (,$(gl_GNULIB_ENABLED_strtoull))
-
-endif
-EXTRA_DIST += strtol.c strtoul.c strtoull.c
-
-EXTRA_libgnu_a_SOURCES += strtol.c strtoul.c strtoull.c
-
-endif
-## end   gnulib module strtoull
-
-## begin gnulib module strtoumax
-ifeq (,$(OMIT_GNULIB_MODULE_strtoumax))
-
-
-EXTRA_DIST += strtoimax.c strtoumax.c
-
-EXTRA_libgnu_a_SOURCES += strtoimax.c strtoumax.c
-
-endif
-## end   gnulib module strtoumax
-
 ## begin gnulib module symlink
 ifeq (,$(OMIT_GNULIB_MODULE_symlink))
 
diff --git a/lib/strtoul.c b/lib/strtoul.c
deleted file mode 100644
index c4974e0..0000000
--- a/lib/strtoul.c
+++ /dev/null
@@ -1,19 +0,0 @@
-/* Copyright (C) 1991, 1997, 2009-2017 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   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 3 of the License, 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/>.  */
-
-#define UNSIGNED        1
-
-#include "strtol.c"
diff --git a/lib/strtoull.c b/lib/strtoull.c
deleted file mode 100644
index 51ae3ac..0000000
--- a/lib/strtoull.c
+++ /dev/null
@@ -1,26 +0,0 @@
-/* Function to parse an 'unsigned long long int' from text.
-   Copyright (C) 1995-1997, 1999, 2009-2017 Free Software Foundation, Inc.
-   NOTE: The canonical source of this file is maintained with the GNU C
-   Library.  Bugs can be reported to bug-glibc@gnu.org.
-
-   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 3 of the License, or 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/>.  */
-
-#define QUAD 1
-
-#include "strtoul.c"
-
-#ifdef _LIBC
-strong_alias (__strtoull_internal, __strtouq_internal)
-weak_alias (strtoull, strtouq)
-#endif
diff --git a/lib/strtoumax.c b/lib/strtoumax.c
deleted file mode 100644
index dc395d6..0000000
--- a/lib/strtoumax.c
+++ /dev/null
@@ -1,2 +0,0 @@
-#define UNSIGNED 1
-#include "strtoimax.c"
diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4
index 3f196d4..8f53a99 100644
--- a/m4/gnulib-comp.m4
+++ b/m4/gnulib-comp.m4
@@ -140,8 +140,6 @@ AC_DEFUN
   # Code from module string:
   # Code from module strtoimax:
   # Code from module strtoll:
-  # Code from module strtoull:
-  # Code from module strtoumax:
   # Code from module symlink:
   # Code from module sys_select:
   # Code from module sys_stat:
@@ -364,12 +362,6 @@ AC_DEFUN
     gl_PREREQ_STRTOIMAX
   fi
   gl_INTTYPES_MODULE_INDICATOR([strtoimax])
-  gl_FUNC_STRTOUMAX
-  if test $HAVE_DECL_STRTOUMAX = 0 || test $REPLACE_STRTOUMAX = 1; then
-    AC_LIBOBJ([strtoumax])
-    gl_PREREQ_STRTOUMAX
-  fi
-  gl_INTTYPES_MODULE_INDICATOR([strtoumax])
   gl_FUNC_SYMLINK
   if test $HAVE_SYMLINK = 0 || test $REPLACE_SYMLINK = 1; then
     AC_LIBOBJ([symlink])
@@ -420,7 +412,6 @@ AC_DEFUN
   gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=false
   gl_gnulib_enabled_secure_getenv=false
   gl_gnulib_enabled_strtoll=false
-  gl_gnulib_enabled_strtoull=false
   gl_gnulib_enabled_tempname=false
   gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec=false
   func_gl_gnulib_m4code_260941c0e5dc67ec9e87d1fb321c300b ()
@@ -569,18 +560,6 @@ AC_DEFUN
       gl_gnulib_enabled_strtoll=true
     fi
   }
-  func_gl_gnulib_m4code_strtoull ()
-  {
-    if ! $gl_gnulib_enabled_strtoull; then
-      gl_FUNC_STRTOULL
-      if test $HAVE_STRTOULL = 0; then
-        AC_LIBOBJ([strtoull])
-        gl_PREREQ_STRTOULL
-      fi
-      gl_STDLIB_MODULE_INDICATOR([strtoull])
-      gl_gnulib_enabled_strtoull=true
-    fi
-  }
   func_gl_gnulib_m4code_tempname ()
   {
     if ! $gl_gnulib_enabled_tempname; then
@@ -649,9 +628,6 @@ AC_DEFUN
   if { test $HAVE_DECL_STRTOIMAX = 0 || test $REPLACE_STRTOIMAX = 1; } && test $ac_cv_type_long_long_int = yes; then
     func_gl_gnulib_m4code_strtoll
   fi
-  if { test $HAVE_DECL_STRTOUMAX = 0 || test $REPLACE_STRTOUMAX = 1; } && test $ac_cv_type_unsigned_long_long_int = yes; then
-    func_gl_gnulib_m4code_strtoull
-  fi
   if test $HAVE_TIMEGM = 0 || test $REPLACE_TIMEGM = 1; then
     func_gl_gnulib_m4code_5264294aa0a5557541b53c8c741f7f31
   fi
@@ -670,7 +646,6 @@ AC_DEFUN
   AM_CONDITIONAL([gl_GNULIB_ENABLED_6099e9737f757db36c47fa9d9f02e88c], [$gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c])
   AM_CONDITIONAL([gl_GNULIB_ENABLED_secure_getenv], [$gl_gnulib_enabled_secure_getenv])
   AM_CONDITIONAL([gl_GNULIB_ENABLED_strtoll], [$gl_gnulib_enabled_strtoll])
-  AM_CONDITIONAL([gl_GNULIB_ENABLED_strtoull], [$gl_gnulib_enabled_strtoull])
   AM_CONDITIONAL([gl_GNULIB_ENABLED_tempname], [$gl_gnulib_enabled_tempname])
   AM_CONDITIONAL([gl_GNULIB_ENABLED_682e609604ccaac6be382e4ee3a4eaec], [$gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec])
   # End of code from modules
@@ -940,9 +915,6 @@ AC_DEFUN
   lib/strtoimax.c
   lib/strtol.c
   lib/strtoll.c
-  lib/strtoul.c
-  lib/strtoull.c
-  lib/strtoumax.c
   lib/symlink.c
   lib/sys_select.in.h
   lib/sys_stat.in.h
@@ -1051,8 +1023,6 @@ AC_DEFUN
   m4/string_h.m4
   m4/strtoimax.m4
   m4/strtoll.m4
-  m4/strtoull.m4
-  m4/strtoumax.m4
   m4/symlink.m4
   m4/sys_select_h.m4
   m4/sys_socket_h.m4
diff --git a/m4/strtoull.m4 b/m4/strtoull.m4
deleted file mode 100644
index c6b2150..0000000
--- a/m4/strtoull.m4
+++ /dev/null
@@ -1,24 +0,0 @@
-# strtoull.m4 serial 7
-dnl Copyright (C) 2002, 2004, 2006, 2008-2017 Free Software Foundation, Inc.
-dnl This file is free software; the Free Software Foundation
-dnl gives unlimited permission to copy and/or distribute it,
-dnl with or without modifications, as long as this notice is preserved.
-
-AC_DEFUN([gl_FUNC_STRTOULL],
-[
-  AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
-  dnl We don't need (and can't compile) the replacement strtoull
-  dnl unless the type 'unsigned long long int' exists.
-  AC_REQUIRE([AC_TYPE_UNSIGNED_LONG_LONG_INT])
-  if test "$ac_cv_type_unsigned_long_long_int" = yes; then
-    AC_CHECK_FUNCS([strtoull])
-    if test $ac_cv_func_strtoull = no; then
-      HAVE_STRTOULL=0
-    fi
-  fi
-])
-
-# Prerequisites of lib/strtoull.c.
-AC_DEFUN([gl_PREREQ_STRTOULL], [
-  :
-])
diff --git a/m4/strtoumax.m4 b/m4/strtoumax.m4
deleted file mode 100644
index 43ef5b5..0000000
--- a/m4/strtoumax.m4
+++ /dev/null
@@ -1,28 +0,0 @@
-# strtoumax.m4 serial 12
-dnl Copyright (C) 2002-2004, 2006, 2009-2017 Free Software Foundation, Inc.
-dnl This file is free software; the Free Software Foundation
-dnl gives unlimited permission to copy and/or distribute it,
-dnl with or without modifications, as long as this notice is preserved.
-
-AC_DEFUN([gl_FUNC_STRTOUMAX],
-[
-  AC_REQUIRE([gl_INTTYPES_H_DEFAULTS])
-
-  dnl On OSF/1 5.1 with cc, this function is declared but not defined.
-  AC_CHECK_FUNCS_ONCE([strtoumax])
-  AC_CHECK_DECLS_ONCE([strtoumax])
-  if test "$ac_cv_have_decl_strtoumax" = yes; then
-    if test "$ac_cv_func_strtoumax" != yes; then
-      # HP-UX 11.11 has "#define strtoimax(...) ..." but no function.
-      REPLACE_STRTOUMAX=1
-    fi
-  else
-    HAVE_DECL_STRTOUMAX=0
-  fi
-])
-
-# Prerequisites of lib/strtoumax.c.
-AC_DEFUN([gl_PREREQ_STRTOUMAX], [
-  AC_CHECK_DECLS([strtoull])
-  AC_REQUIRE([AC_TYPE_UNSIGNED_LONG_LONG_INT])
-])
diff --git a/src/editfns.c b/src/editfns.c
index 98187df..1dbae8f 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -3851,6 +3851,23 @@ usage: (propertize STRING &rest PROPERTIES)  */)
   return string;
 }
 
+/* Convert the prefix of STR from ASCII decimal digits to a number.
+   Set *STR_END to the address of the first non-digit.  Return the
+   number, or PTRDIFF_MAX on overflow.  Return 0 if there is no number.
+   This is like strtol for ptrdiff_t and base 10 and C locale,
+   except without negative numbers or errno.  */
+
+static ptrdiff_t
+str2num (char *str, char **str_end)
+{
+  ptrdiff_t n = 0;
+  for (; c_isdigit (*str); str++)
+    if (INT_MULTIPLY_WRAPV (n, 10, &n) || INT_ADD_WRAPV (n, *str - '0', &n))
+      n = PTRDIFF_MAX;
+  *str_end = str;
+  return n;
+}
+
 DEFUN ("format", Fformat, Sformat, 1, MANY, 0,
        doc: /* Format a string out of a format-string and arguments.
 The first argument is a format control string.
@@ -4057,17 +4074,16 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	     digits to print after the '.' for floats, or the max.
 	     number of chars to print from a string.  */
 
-	  uintmax_t num;
+	  ptrdiff_t num;
 	  char *num_end;
 	  if (c_isdigit (*format))
 	    {
-	      num = strtoumax (format, &num_end, 10);
+	      num = str2num (format, &num_end);
 	      if (*num_end == '$')
 		{
 		  if (num == 0)
 		    error ("Invalid format field number 0");
-		  n = min (num, PTRDIFF_MAX);
-		  n--;
+		  n = num - 1;
 		  format = num_end + 1;
 		}
 	    }
@@ -4095,15 +4111,15 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	  space_flag &= ! plus_flag;
 	  zero_flag &= ! minus_flag;
 
-	  num = strtoumax (format, &num_end, 10);
+	  num = str2num (format, &num_end);
 	  if (max_bufsize <= num)
 	    string_overflow ();
 	  ptrdiff_t field_width = num;
 
 	  bool precision_given = *num_end == '.';
-	  uintmax_t precision = (precision_given
-				 ? strtoumax (num_end + 1, &num_end, 10)
-				 : UINTMAX_MAX);
+	  ptrdiff_t precision = (precision_given
+				 ? str2num (num_end + 1, &num_end)
+				 : PTRDIFF_MAX);
 	  format = num_end;
 
 	  if (format == end)
@@ -4176,7 +4192,7 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	      /* handle case (precision[n] >= 0) */
 
 	      ptrdiff_t prec = -1;
-	      if (precision_given && precision <= TYPE_MAXIMUM (ptrdiff_t))
+	      if (precision_given)
 		prec = precision;
 
 	      /* lisp_string_width ignores a precision of 0, but GNU
@@ -4424,8 +4440,9 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 		 padding and excess precision.  Deal with excess precision
 		 first.  This happens only when the format specifies
 		 ridiculously large precision.  */
-	      uintmax_t excess_precision = precision - prec;
-	      uintmax_t leading_zeros = 0, trailing_zeros = 0;
+	      ptrdiff_t excess_precision
+		= precision_given ? precision - prec : 0;
+	      ptrdiff_t leading_zeros = 0, trailing_zeros = 0;
 	      if (excess_precision)
 		{
 		  if (float_conversion)
@@ -4451,7 +4468,9 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 
 	      /* Compute the total bytes needed for this item, including
 		 excess precision and padding.  */
-	      uintmax_t numwidth = sprintf_bytes + excess_precision;
+	      ptrdiff_t numwidth;
+	      if (INT_ADD_WRAPV (sprintf_bytes, excess_precision, &numwidth))
+		numwidth = PTRDIFF_MAX;
 	      ptrdiff_t padding
 		= numwidth < field_width ? field_width - numwidth : 0;
 	      if (max_bufsize - sprintf_bytes <= excess_precision
diff --git a/src/lread.c b/src/lread.c
index 368b86e..f849398 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -3495,25 +3495,18 @@ substitute_in_interval (INTERVAL interval, Lisp_Object arg)
 }
 
 \f
-#define LEAD_INT 1
-#define DOT_CHAR 2
-#define TRAIL_INT 4
-#define E_EXP 16
-
-
-/* Convert STRING to a number, assuming base BASE.  Return a fixnum if CP has
-   integer syntax and fits in a fixnum, else return the nearest float if CP has
-   either floating point or integer syntax and BASE is 10, else return nil.  If
-   IGNORE_TRAILING, consider just the longest prefix of CP that has
-   valid floating point syntax.  Signal an overflow if BASE is not 10 and the
-   number has integer syntax but does not fit.  */
+/* Convert STRING to a number, assuming base BASE.  Return a fixnum if
+   STRING has integer syntax and fits in a fixnum, else return the
+   nearest float if STRING has either floating point or integer syntax
+   and BASE is 10, else return nil.  If IGNORE_TRAILING, consider just
+   the longest prefix of STRING that has valid floating point syntax.
+   Signal an overflow if BASE is not 10 and the number has integer
+   syntax but does not fit.  */
 
 Lisp_Object
 string_to_number (char const *string, int base, bool ignore_trailing)
 {
-  int state;
   char const *cp = string;
-  int leading_digit;
   bool float_syntax = 0;
   double value = 0;
 
@@ -3525,15 +3518,23 @@ string_to_number (char const *string, int base, bool ignore_trailing)
   bool signedp = negative || *cp == '+';
   cp += signedp;
 
-  state = 0;
-
-  leading_digit = digit_to_number (*cp, base);
+  enum { INTOVERFLOW = 1, LEAD_INT = 2, DOT_CHAR = 4, TRAIL_INT = 8,
+	 E_EXP = 16 };
+  int state = 0;
+  int leading_digit = digit_to_number (*cp, base);
+  uintmax_t n = leading_digit;
   if (leading_digit >= 0)
     {
       state |= LEAD_INT;
-      do
-	++cp;
-      while (digit_to_number (*cp, base) >= 0);
+      for (int digit; 0 <= (digit = digit_to_number (*++cp, base)); )
+	{
+	  if (INT_MULTIPLY_OVERFLOW (n, base))
+	    state |= INTOVERFLOW;
+	  n *= base;
+	  if (INT_ADD_OVERFLOW (n, digit))
+	    state |= INTOVERFLOW;
+	  n += digit;
+	}
     }
   if (*cp == '.')
     {
@@ -3583,32 +3584,22 @@ string_to_number (char const *string, int base, bool ignore_trailing)
 	}
 
       float_syntax = ((state & (DOT_CHAR|TRAIL_INT)) == (DOT_CHAR|TRAIL_INT)
-		      || state == (LEAD_INT|E_EXP));
+		      || (state & ~INTOVERFLOW) == (LEAD_INT|E_EXP));
     }
 
   /* Return nil if the number uses invalid syntax.  If IGNORE_TRAILING, accept
      any prefix that matches.  Otherwise, the entire string must match.  */
   if (! (ignore_trailing
 	 ? ((state & LEAD_INT) != 0 || float_syntax)
-	 : (!*cp && ((state & ~DOT_CHAR) == LEAD_INT || float_syntax))))
+	 : (!*cp && ((state & ~(INTOVERFLOW | DOT_CHAR)) == LEAD_INT
+		     || float_syntax))))
     return Qnil;
 
   /* If the number uses integer and not float syntax, and is in C-language
      range, use its value, preferably as a fixnum.  */
   if (leading_digit >= 0 && ! float_syntax)
     {
-      uintmax_t n;
-
-      /* Fast special case for single-digit integers.  This also avoids a
-	 glitch when BASE is 16 and IGNORE_TRAILING, because in that
-	 case some versions of strtoumax accept numbers like "0x1" that Emacs
-	 does not allow.  */
-      if (digit_to_number (string[signedp + 1], base) < 0)
-	return make_number (negative ? -leading_digit : leading_digit);
-
-      errno = 0;
-      n = strtoumax (string + signedp, NULL, base);
-      if (errno == ERANGE)
+      if (state & INTOVERFLOW)
 	{
 	  /* Unfortunately there's no simple and accurate way to convert
 	     non-base-10 numbers that are out of C-language range.  */
-- 
2.9.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Limit-format-fields-to-more-POSIX-like-spec.patch --]
[-- Type: text/x-patch; name="0002-Limit-format-fields-to-more-POSIX-like-spec.patch", Size: 5754 bytes --]

From 4c9bbc524171fa1022e0cbf4516827be98ae08a5 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 1 Jun 2017 16:03:12 -0700
Subject: [PATCH 2/2] Limit format fields to more POSIX-like spec
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* doc/lispref/strings.texi (Formatting Strings):
Don’t allow mixing numbered with unnumbered format specs.
* src/editfns.c (styled_format): Don’t bother checking for field 0,
since it doesn’t crash and the behavior is not specified.
* test/src/editfns-tests.el (format-with-field): Adjust tests to
match current doc.  Add more tests for out-of-range fields.
---
 doc/lispref/strings.texi  | 13 +++++--------
 src/editfns.c             |  8 ++++----
 test/src/editfns-tests.el | 28 ++++++++++++++++++++++------
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/doc/lispref/strings.texi b/doc/lispref/strings.texi
index 4d33e55..e80e778 100644
--- a/doc/lispref/strings.texi
+++ b/doc/lispref/strings.texi
@@ -965,16 +965,13 @@ Formatting Strings
 decimal number immediately after the initial @samp{%}, followed by a
 literal dollar sign @samp{$}.  It causes the format specification to
 convert the argument with the given number instead of the next
-argument.  Argument 1 is the argument just after the format.
-
-  You can mix specifications with and without field numbers.  A
-specification without a field number that follows a specification with
-a field number will convert the argument after the one specified by
-the field number:
+argument.  Field numbers start at 1.  A format can contain either
+numbered or unnumbered format specifications but not both, except that
+@samp{%%} can be mixed with numbered specifications.
 
 @example
-(format "Argument %2$s, then %s, then %1$s" "x" "y" "z")
-     @result{} "Argument y, then z, then x"
+(format "%2$s, %3$s, %%, %1$s" "x" "y" "z")
+     @result{} "y, z, %, x"
 @end example
 
 @cindex flags in format specifications
diff --git a/src/editfns.c b/src/editfns.c
index 1dbae8f..29af25a 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -3900,8 +3900,10 @@ where field is [0-9]+ followed by a literal dollar "$", flags is
 [+ #-0]+, width is [0-9]+, and precision is a literal period "."
 followed by [0-9]+.
 
-If field is given, it must be a one-based argument number; the given
-argument is substituted instead of the next one.
+If a %-sequence is numbered with a field with positive value N, the
+Nth argument is substituted instead of the next one.  A format can
+contain either numbered or unnumbered %-sequences but not both, except
+that %% can be mixed with numbered %-sequences.
 
 The + flag character inserts a + before any positive number, while a
 space inserts a space before any positive number; these flags only
@@ -4081,8 +4083,6 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
 	      num = str2num (format, &num_end);
 	      if (*num_end == '$')
 		{
-		  if (num == 0)
-		    error ("Invalid format field number 0");
 		  n = num - 1;
 		  format = num_end + 1;
 		}
diff --git a/test/src/editfns-tests.el b/test/src/editfns-tests.el
index c5923aa..54fb743 100644
--- a/test/src/editfns-tests.el
+++ b/test/src/editfns-tests.el
@@ -178,17 +178,33 @@ transpose-test-get-byte-positions
                                (concat (make-string 2048 ?X) "0")))))
 
 (ert-deftest format-with-field ()
-  (should (equal (format "First argument %2$s, then %s, then %1$s" 1 2 3)
+  (should (equal (format "First argument %2$s, then %3$s, then %1$s" 1 2 3)
                  "First argument 2, then 3, then 1"))
-  (should (equal (format "a %2$s %d %1$d %2$S %d %d b" 11 "22" 33 44)
+  (should (equal (format "a %2$s %3$d %1$d %2$S %3$d %4$d b" 11 "22" 33 44)
                  "a 22 33 11 \"22\" 33 44 b"))
-  (should (equal (format "a %08$s %s b" 1 2 3 4 5 6 7 8 9) "a 8 9 b"))
+  (should (equal (format "a %08$s %0000000000000000009$s b" 1 2 3 4 5 6 7 8 9)
+                 "a 8 9 b"))
   (should (equal (should-error (format "a %999999$s b" 11))
                  '(error "Not enough arguments for format string")))
+  (should (equal (should-error (format "a %2147483647$s b"))
+                 '(error "Not enough arguments for format string")))
+  (should (equal (should-error (format "a %9223372036854775807$s b"))
+                 '(error "Not enough arguments for format string")))
+  (should (equal (should-error (format "a %9223372036854775808$s b"))
+                 '(error "Not enough arguments for format string")))
+  (should (equal (should-error (format "a %18446744073709551615$s b"))
+                 '(error "Not enough arguments for format string")))
+  (should (equal (should-error (format "a %18446744073709551616$s b"))
+                 '(error "Not enough arguments for format string")))
+  (should (equal (should-error
+                  (format (format "a %%%d$d b" most-positive-fixnum)))
+                 '(error "Not enough arguments for format string")))
+  (should (equal (should-error
+                  (format (format "a %%%d$d b" (+ 1.0 most-positive-fixnum))))
+                 '(error "Not enough arguments for format string")))
   (should (equal (should-error (format "a %$s b" 11))
                  '(error "Invalid format operation %$")))
-  (should (equal (should-error (format "a %0$s b" 11))
-                 '(error "Invalid format field number 0")))
-  (should (equal (format "a %1$% %s b" 11) "a % 11 b")))
+  (should (equal (should-error (format "a %-1$s b" 11))
+                 '(error "Invalid format operation %$"))))
 
 ;;; editfns-tests.el ends here
-- 
2.9.4


  reply	other threads:[~2017-06-01 23:20 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-28  5:29 i18n/l10n summary Jean-Christophe Helary
2017-05-28 14:27 ` Drew Adams
2017-05-28 14:36   ` Jean-Christophe Helary
2017-05-28 15:33   ` Eli Zaretskii
2017-06-05 12:55   ` Jean-Christophe Helary
2017-07-17 23:22     ` Jean-Christophe Helary
2017-07-22 12:48       ` Jean-Christophe Helary
2017-07-22 13:06         ` Eli Zaretskii
2017-07-22 13:45           ` Jean-Christophe Helary
2017-07-22 14:08             ` Eli Zaretskii
2017-07-22 23:54               ` Jean-Christophe Helary
2017-07-23 14:39                 ` Eli Zaretskii
2017-07-23 23:29                   ` Jean-Christophe Helary
2017-07-24 14:47                     ` Eli Zaretskii
2017-07-24 15:34                       ` Jean-Christophe Helary
2017-07-24 15:51                         ` Eli Zaretskii
2017-07-24 16:08                           ` Jean-Christophe Helary
2017-07-24 16:29                             ` Eli Zaretskii
2017-07-24 16:48                               ` Jean-Christophe Helary
2017-07-24 16:55                                 ` Eli Zaretskii
2017-05-31 22:18 ` Philipp Stephani
2017-05-31 22:29   ` Jean-Christophe Helary
2017-06-01  5:18   ` Paul Eggert
2017-06-01  8:17     ` Philipp Stephani
2017-06-01 23:20       ` Paul Eggert [this message]
2017-06-02  6:52         ` Philipp Stephani
2017-06-03  8:37           ` Paul Eggert
2017-06-03  9:12             ` Andreas Schwab
2017-06-03  9:34             ` Philipp Stephani
2017-06-04 15:54               ` Paul Eggert
2017-06-04 16:45                 ` Eli Zaretskii
2017-06-04 18:37                   ` Paul Eggert
2017-12-03  5:43                 ` Paul Eggert
2017-06-01 14:21   ` Eli Zaretskii
2017-07-02  1:22   ` Jean-Christophe Helary
2017-07-02  2:34     ` Eli Zaretskii
2017-07-28  0:15 ` Byung-Hee HWANG (황병희, 黃炳熙)
2018-04-25 12:58 ` Jean-Christophe Helary
2018-09-21  4:18   ` Jean-Christophe Helary
     [not found] <<AA86315D-1561-41EB-A349-63100C565E8D@gmail.com>
     [not found] ` <<0aca6c65-4610-44c2-99c4-6cbe7aa68c9a@default>
     [not found]   ` <<83a85xgfo2.fsf@gnu.org>
2017-05-28 21:52     ` Drew Adams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b9d78749-ac21-6c2f-33cf-ef00a6ad65ac@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=emacs-devel@gnu.org \
    --cc=jean.christophe.helary@gmail.com \
    --cc=p.stephani2@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.