Re: [Bug-gnulib] proposed vasnprintf patches for address arithmetic and stack overflow

[Paul Eggert]

Bruno Haible <address@hidden> writes:

> then I would have one more unknown to care about: the case when
> sizeof (ptrdiff_t) > sizeof (size_t).

That is not a strong argument for preferring ssize_t to ptrdiff_t,
since we have the same problem with ssize_t.  There is no guarantee
that ssize_t is the same width as size_t either.

ssize_t is a POSIX notion, not a C-language notion.  It is meant for
POSIX calls like 'read', not for pointer arithmetic.  It shouldn't be
used to implement C-language notions like vasnprintf, as these
functions are fundamentally C-language functions and are independent
of POSIX.


> So the meaning of size_t is related to the size of a single memory
> object, whereas ptrdiff_t is more related to the memory size as a whole.

No, both size_t and ptrdiff_t are related only to the size of a single
memory object.  The behavior is undefined if you subtract pointers
that point to different independent objects.


> it's generally better to always work with _unsigned_ differences
> between pointers, and then 'size_t' is good enough.

I'm not sure I agree with this philosophy in general, but if it is the
philosophy in vasnprintf then ssize_t should be avoided as well, since
its problems are slightly worse than ptrdiff_t's in practice.

How about this patch instead?  It avoids both ptrdiff_t and ssize_t,
and this simplifies the code a bit (and makes it run a bit faster :-).
It uses a new symbol NO_ARG_INDEX to get around the problem that
you mentioned with confusion about SIZE_MAX in an earlier patch
that I proposed along these lines.

2003-11-20  Paul Eggert  <address@hidden>

        Remove dependency on ssize_t in vasnprintf; this removes a few
        minor arbitrary limits, and simplifies the code.

        * printf-parse.h: Do not include <sys/types.h>.
        * printf-parse.c: Likewise.
        * printf-parse.h (NO_ARG_INDEX): New macro.
        (char_directive): Use size_t, not ssize_t.
        * vasnprintf.c (VASNPRINTF): Likewise.
        * printf-parse.c (PRINTF_PARSE): Likewise.
        Remove overflow tests that are now unnecessary.
        * printf-parse.c (SSIZE_MAX): Remove.

Index: printf-parse.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/printf-parse.h,v
retrieving revision 1.3
diff -p -u -r1.3 printf-parse.h
--- printf-parse.h      17 Nov 2003 15:14:22 -0000      1.3
+++ printf-parse.h      20 Nov 2003 08:06:12 -0000
@@ -20,10 +20,6 @@
 
 #include "printf-args.h"
 
-/* Get ssize_t.  */
-#include <sys/types.h>
-
-
 /* Flags */
 #define FLAG_GROUP      1      /* ' flag */
 #define FLAG_LEFT       2      /* - flag */
@@ -32,6 +28,9 @@
 #define FLAG_ALT       16      /* # flag */
 #define FLAG_ZERO      32
 
+/* This represents an arg_index that was not specified.  */
+#define NO_ARG_INDEX ((size_t) -1)
+
 /* A parsed directive.  */
 typedef struct
 {
@@ -40,12 +39,12 @@ typedef struct
   int flags;
   const char* width_start;
   const char* width_end;
-  ssize_t width_arg_index;
+  size_t width_arg_index;
   const char* precision_start;
   const char* precision_end;
-  ssize_t precision_arg_index;
+  size_t precision_arg_index;
   char conversion; /* d i o u x X f e E g G c s p n U % but not C S */
-  ssize_t arg_index;
+  size_t arg_index;
 }
 char_directive;
 
Index: printf-parse.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/printf-parse.c,v
retrieving revision 1.4
diff -p -u -r1.4 printf-parse.c
--- printf-parse.c      17 Nov 2003 15:14:22 -0000      1.4
+++ printf-parse.c      20 Nov 2003 08:06:12 -0000
@@ -29,9 +29,6 @@
 /* Get size_t, NULL.  */
 #include <stddef.h>
 
-/* Get ssize_t.  */
-#include <sys/types.h>
-
 /* Get intmax_t.  */
 #if HAVE_STDINT_H_WITH_UINTMAX
 # include <stdint.h>
@@ -46,10 +43,6 @@
 /* Checked size_t computations.  */
 #include "xsize.h"
 
-#ifndef SSIZE_MAX
-# define SSIZE_MAX ((ssize_t) (SIZE_MAX / 2))
-#endif
-
 #if WIDE_CHAR_VERSION
 # define PRINTF_PARSE wprintf_parse
 # define CHAR_T wchar_t
@@ -69,7 +62,7 @@ int
 PRINTF_PARSE (const CHAR_T *format, DIRECTIVES *d, arguments *a)
 {
   const CHAR_T *cp = format;           /* pointer into format */
-  ssize_t arg_posn = 0;                /* number of regular arguments consumed 
*/
+  size_t arg_posn = 0;         /* number of regular arguments consumed */
   size_t d_allocated;                  /* allocated elements of d->dir */
   size_t a_allocated;                  /* allocated elements of a->arg */
   size_t max_width_length = 0;
@@ -123,7 +116,7 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
       CHAR_T c = *cp++;
       if (c == '%')
        {
-         ssize_t arg_index = -1;
+         size_t arg_index = NO_ARG_INDEX;
          DIRECTIVE *dp = &d->dir[d->count];/* pointer to next directive */
 
          /* Initialize the next directive.  */
@@ -131,11 +124,11 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
          dp->flags = 0;
          dp->width_start = NULL;
          dp->width_end = NULL;
-         dp->width_arg_index = -1;
+         dp->width_arg_index = NO_ARG_INDEX;
          dp->precision_start = NULL;
          dp->precision_end = NULL;
-         dp->precision_arg_index = -1;
-         dp->arg_index = -1;
+         dp->precision_arg_index = NO_ARG_INDEX;
+         dp->arg_index = NO_ARG_INDEX;
 
          /* Test for positional argument.  */
          if (*cp >= '0' && *cp <= '9')
@@ -153,7 +146,7 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
                  if (n == 0)
                    /* Positional argument 0.  */
                    goto error;
-                 if (size_overflow_p (n) || n - 1 > SSIZE_MAX)
+                 if (size_overflow_p (n))
                    /* n too large, would lead to out of memory later.  */
                    goto error;
                  arg_index = n - 1;
@@ -223,18 +216,18 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
                      if (n == 0)
                        /* Positional argument 0.  */
                        goto error;
-                     if (size_overflow_p (n) || n - 1 > SSIZE_MAX)
+                     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;
                    }
                }
-             if (dp->width_arg_index < 0)
+             if (dp->width_arg_index == NO_ARG_INDEX)
                {
                  dp->width_arg_index = arg_posn++;
-                 if (dp->width_arg_index < 0)
-                   /* arg_posn wrapped around at SSIZE_MAX.  */
+                 if (dp->width_arg_index == NO_ARG_INDEX)
+                   /* arg_posn wrapped around.  */
                    goto error;
                }
              REGISTER_ARG (dp->width_arg_index, TYPE_INT);
@@ -280,7 +273,7 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
                          if (n == 0)
                            /* Positional argument 0.  */
                            goto error;
-                         if (size_overflow_p (n) || n - 1 > SSIZE_MAX)
+                         if (size_overflow_p (n))
                            /* n too large, would lead to out of memory
                               later.  */
                            goto error;
@@ -288,11 +281,11 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
                          cp = np + 1;
                        }
                    }
-                 if (dp->precision_arg_index < 0)
+                 if (dp->precision_arg_index == NO_ARG_INDEX)
                    {
                      dp->precision_arg_index = arg_posn++;
-                     if (dp->precision_arg_index < 0)
-                       /* arg_posn wrapped around at SSIZE_MAX.  */
+                     if (dp->precision_arg_index == NO_ARG_INDEX)
+                       /* arg_posn wrapped around.  */
                        goto error;
                    }
                  REGISTER_ARG (dp->precision_arg_index, TYPE_INT);
@@ -491,11 +484,11 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
            if (type != TYPE_NONE)
              {
                dp->arg_index = arg_index;
-               if (dp->arg_index < 0)
+               if (dp->arg_index == NO_ARG_INDEX)
                  {
                    dp->arg_index = arg_posn++;
-                   if (dp->arg_index < 0)
-                     /* arg_posn wrapped around at SSIZE_MAX.  */
+                   if (dp->arg_index == NO_ARG_INDEX)
+                     /* arg_posn wrapped around.  */
                      goto error;
                  }
                REGISTER_ARG (dp->arg_index, type);
@@ -511,9 +504,6 @@ PRINTF_PARSE (const CHAR_T *format, DIRE
              DIRECTIVE *memory;
 
              d_allocated = xtimes (d_allocated, 2);
-             if (size_overflow_p (d_allocated))
-               /* Overflow, would lead to out of memory.  */
-               goto error;
              memory_size = xtimes (d_allocated, sizeof (DIRECTIVE));
              if (size_overflow_p (memory_size))
                /* Overflow, would lead to out of memory.  */
Index: vasnprintf.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/vasnprintf.c,v
retrieving revision 1.11
diff -p -u -r1.11 vasnprintf.c
--- vasnprintf.c        18 Nov 2003 15:29:47 -0000      1.11
+++ vasnprintf.c        20 Nov 2003 08:06:13 -0000
@@ -222,7 +222,7 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
          {
            size_t augmented_length;
 
-           if (!(dp->arg_index < 0))
+           if (!(dp->arg_index == NO_ARG_INDEX))
              abort ();
            augmented_length = xsum (length, 1);
            ENSURE_ALLOCATION (augmented_length);
@@ -231,7 +231,7 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
          }
        else
          {
-           if (!(dp->arg_index >= 0))
+           if (!(dp->arg_index != NO_ARG_INDEX))
              abort ();
 
            if (dp->conversion == 'n')
@@ -279,7 +279,7 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
                  width = 0;
                  if (dp->width_start != dp->width_end)
                    {
-                     if (dp->width_arg_index >= 0)
+                     if (dp->width_arg_index != NO_ARG_INDEX)
                        {
                          int arg;
 
@@ -301,7 +301,7 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
                  precision = 6;
                  if (dp->precision_start != dp->precision_end)
                    {
-                     if (dp->precision_arg_index >= 0)
+                     if (dp->precision_arg_index != NO_ARG_INDEX)
                        {
                          int arg;
 
@@ -563,13 +563,13 @@ VASNPRINTF (CHAR_T *resultbuf, size_t *l
 
                /* Construct the arguments for calling snprintf or sprintf.  */
                prefix_count = 0;
-               if (dp->width_arg_index >= 0)
+               if (dp->width_arg_index != NO_ARG_INDEX)
                  {
                    if (!(a.arg[dp->width_arg_index].type == TYPE_INT))
                      abort ();
                    prefixes[prefix_count++] = 
a.arg[dp->width_arg_index].a.a_int;
                  }
-               if (dp->precision_arg_index >= 0)
+               if (dp->precision_arg_index != NO_ARG_INDEX)
                  {
                    if (!(a.arg[dp->precision_arg_index].type == TYPE_INT))
                      abort ();