bug-gnulib
[Top][All Lists]
Advanced

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

[Bug-gnulib] proposal to decouple vasnprintf from xsize


From: Paul Eggert
Subject: [Bug-gnulib] proposal to decouple vasnprintf from xsize
Date: Sat, 07 Aug 2004 01:08:15 -0700
User-agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux)

I have my qualms about the xsize.h method for size-overflow checking,
and would like a variant of vasnprintf that does overflow checking
only when needed, and does not use operations like "xsum" and
"xtimes".  I realize there are some advantages to the xsize approach,
but I prefer the approach where I see overflow checking as I go.

One way to do this is to apply the following patch.  However, I recall
that Bruno preferred the xsize approach and so we might have to think
of other ideas.  Two other possibilities are to conditionally compile
vasnprintf code to use xsize, so that each vasnprintf-using package
can decide on its own whether to use xsize -- which I could write
another patch for, if you like.  Or we could add a new module that has
a copy of printf-parse.c and vasnprintf.c (basically, with the
following patches applied).

The following patch is already being used by coreutils.

Comments?

2004-08-07  Paul Eggert  <address@hidden>

        Decouple vasnprintf from xsize.
        * modules/vasnprintf: Remove dependency on xsize.
        * lib/printf-parse.c: Do not include "xsize.h".
        (SIZE_MAX): Define if not already defined.
        * lib/vasnprintf.c: Likewise.
        * lib/printf-parse.c (REGISTER_ARG, PRINTF_PARSE):
        Use explicit checks for overflow instead of the xsize macros.
        * lib/vasnprintf.c (VASNPRINTF, ENSURE_ALLOCATION): Likewise.
        (ENSURE_ALLOCATION): Arg is now the length increment, not the
        new length; this lets us check the length and simplifies the callers.

Index: modules/vasnprintf
===================================================================
RCS file: /cvsroot/gnulib/gnulib/modules/vasnprintf,v
retrieving revision 1.3
diff -p -u -r1.3 vasnprintf
--- modules/vasnprintf  25 Nov 2003 11:18:47 -0000      1.3
+++ modules/vasnprintf  7 Aug 2004 07:57:31 -0000
@@ -19,7 +19,6 @@ m4/vasnprintf.m4
 
 Depends-on:
 alloca
-xsize
 
 configure.ac:
 gl_FUNC_VASNPRINTF
Index: lib/printf-parse.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/printf-parse.c,v
retrieving revision 1.5
diff -p -u -r1.5 printf-parse.c
--- lib/printf-parse.c  25 Nov 2003 11:18:46 -0000      1.5
+++ lib/printf-parse.c  7 Aug 2004 07:57:30 -0000
@@ -1,5 +1,5 @@
 /* Formatted output to strings.
-   Copyright (C) 1999-2000, 2002-2003 Free Software Foundation, Inc.
+   Copyright (C) 1999-2000, 2002-2004 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
@@ -40,8 +40,9 @@
 /* malloc(), realloc(), free().  */
 #include <stdlib.h>
 
-/* Checked size_t computations.  */
-#include "xsize.h"
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
 
 #if WIDE_CHAR_VERSION
 # define PRINTF_PARSE wprintf_parse
@@ -87,13 +88,13 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
        size_t memory_size;                                             \
        argument *memory;                                               \
                                                                        \
-       a_allocated = xtimes (a_allocated, 2);                          \
+       a_allocated *= 2;                                               \
        if (a_allocated <= n)                                           \
-         a_allocated = xsum (n, 1);                                    \
-       memory_size = xtimes (a_allocated, sizeof (argument));          \
-       if (size_overflow_p (memory_size))                              \
+         a_allocated = n + 1;                                          \
+       if (SIZE_MAX / sizeof (argument) < a_allocated)                 \
          /* Overflow, would lead to out of memory.  */                 \
          goto error;                                                   \
+       memory_size = a_allocated * sizeof (argument);                  \
        memory = (a->arg                                                \
                  ? realloc (a->arg, memory_size)                       \
                  : malloc (memory_size));                              \
@@ -142,13 +143,14 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
                  size_t n = 0;
 
                  for (np = cp; *np >= '0' && *np <= '9'; np++)
-                   n = xsum (xtimes (n, 10), *np - '0');
+                   if (n < SIZE_MAX / 10)
+                     n = 10 * n + (*np - '0');
+                   else
+                     /* n too large for memory.  */
+                     goto error;
                  if (n == 0)
                    /* Positional argument 0.  */
                    goto error;
-                 if (size_overflow_p (n))
-                   /* n too large, would lead to out of memory later.  */
-                   goto error;
                  arg_index = n - 1;
                  cp = np + 1;
                }
@@ -212,13 +214,14 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
                      size_t n = 0;
 
                      for (np = cp; *np >= '0' && *np <= '9'; np++)
-                       n = xsum (xtimes (n, 10), *np - '0');
+                       if (n < SIZE_MAX / 10)
+                         n = 10 * n + (*np - '0');
+                       else
+                         /* n too large for memory.  */
+                         goto error;
                      if (n == 0)
                        /* Positional argument 0.  */
                        goto error;
-                     if (size_overflow_p (n))
-                       /* n too large, would lead to out of memory later.  */
-                       goto error;
                      dp->width_arg_index = n - 1;
                      cp = np + 1;
                    }
@@ -269,14 +272,14 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
                          size_t n = 0;
 
                          for (np = cp; *np >= '0' && *np <= '9'; np++)
-                           n = xsum (xtimes (n, 10), *np - '0');
+                           if (n < SIZE_MAX / 10)
+                             n = 10 * n + (*np - '0');
+                           else
+                             /* n too large for memory.  */
+                             goto error;
                          if (n == 0)
                            /* Positional argument 0.  */
                            goto error;
-                         if (size_overflow_p (n))
-                           /* n too large, would lead to out of memory
-                              later.  */
-                           goto error;
                          dp->precision_arg_index = n - 1;
                          cp = np + 1;
                        }
@@ -500,15 +503,13 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
          d->count++;
          if (d->count >= d_allocated)
            {
-             size_t memory_size;
              DIRECTIVE *memory;
 
-             d_allocated = xtimes (d_allocated, 2);
-             memory_size = xtimes (d_allocated, sizeof (DIRECTIVE));
-             if (size_overflow_p (memory_size))
+             if (SIZE_MAX / (2 * sizeof (DIRECTIVE)) < d_allocated)
                /* Overflow, would lead to out of memory.  */
                goto error;
-             memory = realloc (d->dir, memory_size);
+             d_allocated *= 2;
+             memory = realloc (d->dir, d_allocated * sizeof (DIRECTIVE));
              if (memory == NULL)
                /* Out of memory.  */
                goto error;
Index: lib/vasnprintf.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/vasnprintf.c,v
retrieving revision 1.13
diff -p -u -r1.13 vasnprintf.c
--- lib/vasnprintf.c    17 May 2004 11:27:08 -0000      1.13
+++ lib/vasnprintf.c    7 Aug 2004 07:57:31 -0000
@@ -48,8 +48,9 @@
 # include "printf-parse.h"
 #endif
 
-/* Checked size_t computations.  */
-#include "xsize.h"
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
 
 #ifdef HAVE_WCHAR_T
 # ifdef HAVE_WCSLEN
@@ -143,8 +144,7 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
 
     /* Allocate a small buffer that will hold a directive passed to
        sprintf or snprintf.  */
-    buf_neededlength =
-      xsum4 (7, d.max_width_length, d.max_precision_length, 6);
+    buf_neededlength = 7 + d.max_width_length + d.max_precision_length + 6;
 #if HAVE_ALLOCA
     if (buf_neededlength < 4000 / sizeof (CHAR_T))
       {
@@ -154,10 +154,9 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
     else
 #endif
       {
-       size_t buf_memsize = xtimes (buf_neededlength, sizeof (CHAR_T));
-       if (size_overflow_p (buf_memsize))
+       if (SIZE_MAX / sizeof (CHAR_T) < buf_neededlength)
          goto out_of_memory_1;
-       buf = (CHAR_T *) malloc (buf_memsize);
+       buf = (CHAR_T *) malloc (buf_neededlength * sizeof (CHAR_T));
        if (buf == NULL)
          goto out_of_memory_1;
        buf_malloced = buf;
@@ -178,20 +177,24 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
        result is either == resultbuf or == NULL or malloc-allocated.
        If length > 0, then result != NULL.  */
 
-    /* Ensures that allocated >= needed.  Aborts through a jump to
-       out_of_memory if needed is SIZE_MAX or otherwise too big.  */
-#define ENSURE_ALLOCATION(needed) \
-    if ((needed) > allocated)                                               \
+    /* Ensures that allocated >= length + extra.  Aborts through a jump to
+       out_of_memory if size is too big.  */
+#define ENSURE_ALLOCATION(extra) \
+  {                                                                         \
+    size_t needed = length + (extra);                                       \
+    if (needed < length)                                                    \
+      goto out_of_memory;                                                   \
+    if (needed > allocated)                                                 \
       {                                                                        
     \
        size_t memory_size;                                                  \
        CHAR_T *memory;                                                      \
                                                                             \
-       allocated = (allocated > 0 ? xtimes (allocated, 2) : 12);            \
-       if ((needed) > allocated)                                            \
-         allocated = (needed);                                              \
-       memory_size = xtimes (allocated, sizeof (CHAR_T));                   \
-       if (size_overflow_p (memory_size))                                   \
+       allocated = (allocated > 0 ? 2 * allocated : 12);                    \
+       if (needed > allocated)                                              \
+         allocated = needed;                                                \
+       if (SIZE_MAX / sizeof (CHAR_T) < allocated)                          \
          goto out_of_memory;                                                \
+       memory_size = allocated * sizeof (CHAR_T);                           \
        if (result == resultbuf || result == NULL)                           \
          memory = (CHAR_T *) malloc (memory_size);                          \
        else                                                                 \
@@ -201,18 +204,18 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
        if (result == resultbuf && length > 0)                               \
          memcpy (memory, result, length * sizeof (CHAR_T));                 \
        result = memory;                                                     \
-      }
+      }                                                                        
     \
+  }
 
     for (cp = format, i = 0, dp = &d.dir[0]; ; cp = dp->dir_end, i++, dp++)
       {
        if (cp != dp->dir_start)
          {
            size_t n = dp->dir_start - cp;
-           size_t augmented_length = xsum (length, n);
 
-           ENSURE_ALLOCATION (augmented_length);
+           ENSURE_ALLOCATION (n);
            memcpy (result + length, cp, n * sizeof (CHAR_T));
-           length = augmented_length;
+           length += n;
          }
        if (i == d.count)
          break;
@@ -220,14 +223,11 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
        /* Execute a single directive.  */
        if (dp->conversion == '%')
          {
-           size_t augmented_length;
-
            if (!(dp->arg_index == ARG_NONE))
              abort ();
-           augmented_length = xsum (length, 1);
-           ENSURE_ALLOCATION (augmented_length);
+           ENSURE_ALLOCATION (1);
            result[length] = '%';
-           length = augmented_length;
+           length += 1;
          }
        else
          {
@@ -293,7 +293,11 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
                          const CHAR_T *digitp = dp->width_start;
 
                          do
-                           width = xsum (xtimes (width, 10), *digitp++ - '0');
+                           {
+                             if (SIZE_MAX / 10 <= width)
+                               goto out_of_memory;
+                             width = width * 10 + (*digitp++ - '0');
+                           }
                          while (digitp != dp->width_end);
                        }
                    }
@@ -316,7 +320,12 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
 
                          precision = 0;
                          while (digitp != dp->precision_end)
-                           precision = xsum (xtimes (precision, 10), *digitp++ 
- '0');
+                           {
+                             size_t p1 = 10 * precision + (*digitp++ - '0');
+                             precision = ((SIZE_MAX / 10 < precision
+                                           || p1 < precision)
+                                          ? SIZE_MAX : p1);
+                           }
                        }
                    }
 
@@ -426,14 +435,18 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
                                         )
                          + 1 /* turn floor into ceil */
                          + 10; /* sign, decimal point etc. */
-                     tmp_length = xsum (tmp_length, precision);
+                     tmp_length += precision;
+                     if (tmp_length < precision)
+                       goto out_of_memory;
                      break;
 
                    case 'e': case 'E': case 'g': case 'G':
                    case 'a': case 'A':
                      tmp_length =
                        12; /* sign, decimal point, exponent etc. */
-                     tmp_length = xsum (tmp_length, precision);
+                     tmp_length += precision;
+                     if (tmp_length < precision)
+                       goto out_of_memory;
                      break;
 
                    case 'c':
@@ -453,7 +466,9 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
                            local_wcslen (a.arg[dp->arg_index].a.a_wide_string);
 
 #  if !WIDE_CHAR_VERSION
-                         tmp_length = xtimes (tmp_length, MB_CUR_MAX);
+                         if (SIZE_MAX / MB_CUR_MAX < tmp_length)
+                           goto out_of_memory;
+                         tmp_length *= MB_CUR_MAX;
 #  endif
                        }
                      else
@@ -477,19 +492,19 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
                  if (tmp_length < width)
                    tmp_length = width;
 
-                 tmp_length = xsum (tmp_length, 1); /* account for trailing 
NUL */
+                 tmp_length++; /* account for trailing NUL */
+                 if (!tmp_length)
+                   goto out_of_memory;
                }
 
                if (tmp_length <= sizeof (tmpbuf) / sizeof (CHAR_T))
                  tmp = tmpbuf;
                else
                  {
-                   size_t tmp_memsize = xtimes (tmp_length, sizeof (CHAR_T));
-
-                   if (size_overflow_p (tmp_memsize))
+                   if (SIZE_MAX / sizeof (CHAR_T) < tmp_length)
                      /* Overflow, would lead to out of memory.  */
                      goto out_of_memory;
-                   tmp = (CHAR_T *) malloc (tmp_memsize);
+                   tmp = (CHAR_T *) malloc (tmp_length * sizeof (CHAR_T));
                    if (tmp == NULL)
                      /* Out of memory.  */
                      goto out_of_memory;
@@ -578,7 +593,7 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
 #if USE_SNPRINTF
                /* Prepare checking whether snprintf returns the count
                   via %n.  */
-               ENSURE_ALLOCATION (xsum (length, 1));
+               ENSURE_ALLOCATION (1);
                result[length] = '\0';
 #endif
 
@@ -784,7 +799,7 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
                                   that would have been required) when the
                                   buffer is too small.  */
                                size_t bigger_need =
-                                 xsum (xtimes (allocated, 2), 12);
+                                 (allocated > 12 ? allocated : 12);
                                ENSURE_ALLOCATION (bigger_need);
                                continue;
                              }
@@ -819,10 +834,8 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
                        /* Need at least count bytes.  But allocate
                           proportionally, to avoid looping eternally if
                           snprintf() reports a too small count.  */
-                       size_t n =
-                         xmax (xsum (length, count), xtimes (allocated, 2));
-
-                       ENSURE_ALLOCATION (n);
+                       ENSURE_ALLOCATION (count < allocated
+                                          ? allocated : count);
 #if USE_SNPRINTF
                        continue;
 #endif
@@ -845,7 +858,7 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
       }
 
     /* Add the final NUL.  */
-    ENSURE_ALLOCATION (xsum (length, 1));
+    ENSURE_ALLOCATION (1);
     result[length] = '\0';
 
     if (result != resultbuf && length + 1 < allocated)




reply via email to

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