Skip site navigation (1)Skip section navigation (2)
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>