[Top][All Lists]
[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)
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Bug-gnulib] proposal to decouple vasnprintf from xsize,
Paul Eggert <=