Date: Wed, 20 Dec 2000 10:55:36 +0000 From: Ben Smithurst <ben@FreeBSD.org> To: audit@FreeBSD.ORG Subject: Re: printf(1) broken for some long format strings Message-ID: <20001220105536.A8350@strontium.scientia.demon.co.uk> In-Reply-To: <20001220124232.I54775@gsmx07.alcatel.com.au> References: <20001219143506.C78749@strontium.scientia.demon.co.uk> <200012191729.eBJHTps36903@billy-club.village.org> <20001219215413.H78749@strontium.scientia.demon.co.uk> <20001220124232.I54775@gsmx07.alcatel.com.au>
next in thread | previous in thread | raw e-mail | index | archive | help
Peter Jeremy wrote:
> One problem with your patch is that it just ensures that there is
> sufficient space for the current copy. This means that realloc() is
> likely to be called a number of times with slightly larger lengths
> each time. This sort of behaviour is likely to lead to memory
> fragmentation, which may be significant in long running processes
> (like interactive shells). Over-estimating the amount of memory
> needed when actually allocating memory will reduce the number of
> realloc()s and hence the amount of fragmentation.
ok, but I don't think realloc() will be needed many times if we just
round to the nearest k boundary. I think the buffer in mklong() is only
needed when there's a long string with no format strings in it followed
by a format like "%d", and it's probably fairly rare for printf(1) to be
used with a string like that over 1k in length. Unless anyone knows
otherwise.
So, my final proposal. :-) I stopped using reallocf() even though Bruce
wanted it, because we use ckrealloc() in the shell, the overall code is
actually a bit simpler without reallocf().
Index: printf.c
===================================================================
RCS file: /usr/cvs/src/usr.bin/printf/printf.c,v
retrieving revision 1.15
diff -u -r1.15 printf.c
--- printf.c 2000/09/04 06:11:25 1.15
+++ printf.c 2000/12/19 22:15:31
@@ -60,6 +60,7 @@
#ifdef SHELL
#define main printfcmd
#include "bltin/bltin.h"
+#include "memalloc.h"
#else
#define warnx1(a, b, c) warnx(a)
#define warnx2(a, b, c) warnx(a, b)
@@ -247,12 +248,23 @@
char *str;
int ch;
{
- static char copy[64];
- int len;
+ static char *copy;
+ static size_t copy_size;
+ size_t len, newlen;
+ char *newcopy;
len = strlen(str) + 2;
- if (len > sizeof copy)
- return NULL;
+ if (len > copy_size) {
+ newlen = ((len + 1023) >> 10) << 10;
+#ifdef SHELL
+ if ((newcopy = ckrealloc(copy, newlen)) == NULL)
+#else
+ if ((newcopy = realloc(copy, newlen)) == NULL)
+#endif
+ return (NULL);
+ copy = newcopy;
+ copy_size = newlen;
+ }
memmove(copy, str, len - 3);
copy[len - 3] = 'q';
--
Ben Smithurst / ben@FreeBSD.org / PGP: 0x99392F7D
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001220105536.A8350>