From owner-cvs-all@FreeBSD.ORG Sun Apr 15 08:30:09 2007 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 4A4C216A417; Sun, 15 Apr 2007 08:30:09 +0000 (UTC) (envelope-from stas@FreeBSD.org) Received: from com1.ht-systems.ru (com1.ht-systems.ru [83.97.104.204]) by mx1.freebsd.org (Postfix) with ESMTP id CCA9A13C44C; Sun, 15 Apr 2007 08:30:08 +0000 (UTC) (envelope-from stas@FreeBSD.org) Received: from [83.97.105.114] (helo=phonon.SpringDaemons.com ident=postfix) by com1.ht-systems.ru with esmtpa (Exim 4.62) (envelope-from ) id 1Hd07Z-0007yf-Mv; Sun, 15 Apr 2007 12:30:06 +0400 Received: from localhost (localhost [127.0.0.1]) by phonon.SpringDaemons.com (Postfix) with SMTP id 22D1511497; Sun, 15 Apr 2007 12:28:05 +0400 (MSD) Date: Sun, 15 Apr 2007 12:28:05 +0400 From: Stanislav Sedov To: Bruce Evans Message-Id: <20070415122805.96ebce64.stas@FreeBSD.org> In-Reply-To: <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> <20070415161007.G37658@besplex.bde.org> Organization: The FreeBSD Project X-Mailer: carrier-pigeon X-Voice: +7 916 849 20 23 X-XMPP: ssedov@jabber.ru X-ICQ: 208105021 X-Yahoo: stanislav_sedov X-PGP-Fingerprint: F21E D6CC 5626 9609 6CE2 A385 2BF5 5993 EB26 9581 X-University: MEPhI Mime-Version: 1.0 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg="PGP-SHA1"; boundary="Signature=_Sun__15_Apr_2007_12_28_05_+0400_tH.UCQ5SvbeGfUuB" X-Spam-Flag: SKIP Cc: src-committers@freebsd.org, cvs-src@freebsd.org, cvs-all@freebsd.org, Stanislav Sedov , pav@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-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 15 Apr 2007 08:30:09 -0000 --Signature=_Sun__15_Apr_2007_12_28_05_+0400_tH.UCQ5SvbeGfUuB Content-Type: text/plain; charset=US-ASCII Content-Disposition: inline Content-Transfer-Encoding: 7bit On Sun, 15 Apr 2007 17:25:15 +1000 (EST) Bruce Evans mentioned: > 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. > Yeah, already discovered too. > 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. > Thanks for deep analysis, I'll try to optimise and cleanup the code to make it more smart. BTW, I've modified it to use dynamically allocated buffers (I've forgotten to CC it to cvs@freebsd.org). It allocate buffers for cached strings of (screen_width + 1) bytes length now to be able to store the full screen line and terminating NULL. Can you review, please? -- Stanislav Sedov ST4096-RIPE --Signature=_Sun__15_Apr_2007_12_28_05_+0400_tH.UCQ5SvbeGfUuB Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (FreeBSD) iD8DBQFGIeIVK/VZk+smlYERAom9AJ49pS2WmJLzQZOdmjpm07MSDt7rLgCffTtU XQhddJSU85+WtPCH1LfqypQ= =tk3D -----END PGP SIGNATURE----- --Signature=_Sun__15_Apr_2007_12_28_05_+0400_tH.UCQ5SvbeGfUuB--