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>