From owner-cvs-src@FreeBSD.ORG Sun Apr 15 07:25:22 2007 Return-Path: X-Original-To: cvs-src@freebsd.org Delivered-To: cvs-src@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id CB3E716A400; Sun, 15 Apr 2007 07:25:22 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailout2.pacific.net.au (mailout2-3.pacific.net.au [61.8.2.226]) by mx1.freebsd.org (Postfix) with ESMTP id 66D8B13C45B; Sun, 15 Apr 2007 07:25:22 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailproxy2.pacific.net.au (mailproxy2.pacific.net.au [61.8.2.163]) by mailout2.pacific.net.au (Postfix) with ESMTP id EC20710A13A; Sun, 15 Apr 2007 17:25:12 +1000 (EST) Received: from besplex.bde.org (katana.zip.com.au [61.8.7.246]) by mailproxy2.pacific.net.au (Postfix) with ESMTP id 0088927412; Sun, 15 Apr 2007 17:25:16 +1000 (EST) Date: Sun, 15 Apr 2007 17:25:15 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Stanislav Sedov In-Reply-To: <20070414184627.746fa559.stas@FreeBSD.org> Message-ID: <20070415161007.G37658@besplex.bde.org> References: <200704141016.l3EAGqIs023798@repoman.freebsd.org> <1176546388.54822.11.camel@ikaros.oook.cz> <1176546959.54822.14.camel@ikaros.oook.cz> <20070414154246.89ad2946.stas@FreeBSD.org> <20070414124654.GB1687@zaphod.nitro.dk> <20070414181730.eca262c0.stas@FreeBSD.org> <20070414184627.746fa559.stas@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: cvs-src@freebsd.org, pav@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org, "Simon L. Nielsen" Subject: Re: cvs commit: src/contrib/top top.X top.c top.h src/usr.bin/top machine.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 15 Apr 2007 07:25:22 -0000 On Sat, 14 Apr 2007, Stanislav Sedov wrote: > On Sat, 14 Apr 2007 18:17:30 +0400 > Stanislav Sedov mentioned: > >> Well, not quite right. If you screen is wider then 128 symbols, there >> could be an overflow, since the row buffer is 128 bytes length. >> >> I have not touched any limits, just replaced the string it displays. So >> there can be overflow with patch or without it, if both the command >> name and screen width is wider then 128. It couldn't overflow before, because the command name width was limited by sizeof(pp->ki_comm) = 20, and other field widths are mostly fixed and must add up to less than about 60 to leave space for almost 20-character command names on 80-column terminals. > That's even better. In contrib src display.c it always NULL-terminate > string at the screen_width point, so it's always write '\0' to the > unknown area if screen_width is larger than 128. Hopefully, the storage > is allocated last in .data section, so it doesn't overwrite any > important data and code. No, it NUL-terminates at display_width = min(screen_width, MAX_COLS - 1). display_width's only reason for existence is to cache the result of this min() so as to do the truncation efficiently as well as safely. MAX_COLS is bogusly named. It is the buffer size for various buffers. It is certainly not the maximum nomber of columns, but is effectively 1 less than that, since if the physical display width is > MAX_COLS - 1 then the truncation has the non-null effect of limiting all output to MAX_COLS - 1 columns. screen_width is slightly bogusly named. It is normally the physical screen width less 1. Here 1 is subtracted to avoid problems with line wrap, so this name is OK if you think of the screen width as being the usable screen width, but that width as actually given by the display_width variable so the screen_width variable shouldn't be used for it too. I think screen_width should be the physical screen width and private to screen.c and maybe display.c. The only correct use of screen_width is currently to initialize display_width from it. Other uses give the buffer overrun in top.c and completely wrong right justification of some things in display.c in cases where screen_width != display_width. Right justification to display_width would be less wrong. The interaction of the subtraction of 1 from screen_width with the subtraction of 1 from MAX_COLS is a bit confusing and gives potential buffer overrun for the default initialization of screen_width to MAX_COLS (should be to MAX_COLS - 1). This initialization is used if top can't figure out the terminal width. Then it has the same effect as a terminal of width MAX_COLS + 1 -- there is no problem in display.c since display_width = MAX_COLS - 1 is used there, but the printf() in top.c can overrun by 1 now that the command name width can be large. Bruce