[Patches] Fix for bug PR#341
M.-A. Lemburg
mal@lemburg.com
Fri, 02 Jun 2000 10:53:13 +0200
Trent Mick wrote:
>
> Discussion:
>
> This patch fixes the string formatting overflow problem. It tries to do a
> little better than MAL's magic nunumber (50) check. If this looks good then I
> can do the same for unicodeobject.c
>
> [Tim P on MAL's original patch]
> > but I'll join Fred in objecting to the code
> > it's mimicking: not only do magic numbers suck, but these particular magic
> > numbers implicitly rely on PyString_Format's tmpbuf vector being declared of
> > another magical size larger than them. As usual, flaky code gets flakier.
>
> My patch still uses the magic number for the temporary buffer. This seems to me
> a good practical limit. With the patch this buffer can no longer overflow (as
> well, it is faster than malloc'ing a perfect sized buffer every time).
>
> [MAL]
> > A redesign would, of course, use a malloced buffer, the n-variants
> > of printf() and add long support ;-) ... maybe for 1.7.
>
> No long support in this patch :(
>
> [Guido on MAL's original patch]
> > Having read the patch and the discussion about magic numbers, I agree
> > with Marc-Andre: let's apply the quick fix now, worry about
> > correctness later.
>
> Maybe this patch is preferable.
Yep. The patch doesn't yet address the general case (use malloc etc.),
but makes things a little more flexbile.
(Even though I find some tests size_t vs. INT_MAX overkill --
noone will store 2GB strings in memory ;-)
> int sign;
> int len;
> ! #define TMPBUFLEN (size_t)120
> ! char tmpbuf[TMPBUFLEN]; /* For format{float,int,char}() */
> char *fmt_start = fmt;
You should move this #define outside of the function for
better visibility (and perhaps add a comment).
> fmt++;
> ***************
> *** 2618,2624 ****
> if (c == 'i')
> c = 'd';
> buf = tmpbuf;
> ! len = formatint(buf, flags, prec, c, v);
> if (len < 0)
> goto error;
> sign = (c == 'd');
> --- 2653,2659 ----
> if (c == 'i')
> c = 'd';
> buf = tmpbuf;
> ! len = formatint(buf, TMPBUFLEN, flags, prec, c, v);
All of these direct references to TMPBUFLEN should
be changed to sizeof(buf) for clarity.
--
Marc-Andre Lemburg
______________________________________________________________________
Business: http://www.lemburg.com/
Python Pages: http://www.lemburg.com/python/