From owner-svn-src-head@freebsd.org Wed Apr 3 13:17:12 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 91DC4156DC72; Wed, 3 Apr 2019 13:17:12 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id F2FE780AA7; Wed, 3 Apr 2019 13:17:11 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 801DF4388E1; Thu, 4 Apr 2019 00:17:00 +1100 (AEDT) Date: Thu, 4 Apr 2019 00:16:58 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Dimitry Andric cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r345807 - head/usr.bin/top In-Reply-To: <201904021801.x32I1sxX019439@repo.freebsd.org> Message-ID: <20190403234558.X1970@besplex.bde.org> References: <201904021801.x32I1sxX019439@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=FNpr/6gs c=1 sm=1 tr=0 cx=a_idp_d a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=qgxEQ-H3AAAA:8 a=ilqspKNxh9cREQBrOfMA:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 a=p3XWM3U2PI-Bt-EJryuK:22 X-Rspamd-Queue-Id: F2FE780AA7 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.96 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.96)[-0.958,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Apr 2019 13:17:12 -0000 On Tue, 2 Apr 2019, Dimitry Andric wrote: > Author: dim > Date: Tue Apr 2 18:01:54 2019 > New Revision: 345807 > URL: https://svnweb.freebsd.org/changeset/base/345807 > > Log: > Fix regression in top(1) after r344381, causing informational messages > to no longer be displayed. This was because the reimplementation of > setup_buffer() did not copy the previous contents into any reallocated > buffer. > > Reported by: James Wright > PR: 236947 > MFC after: 3 days > > Modified: > head/usr.bin/top/display.c > > Modified: head/usr.bin/top/display.c > ============================================================================== > --- head/usr.bin/top/display.c Tue Apr 2 17:51:28 2019 (r345806) > +++ head/usr.bin/top/display.c Tue Apr 2 18:01:54 2019 (r345807) > @@ -1347,7 +1347,8 @@ i_uptime(struct timeval *bt, time_t *tod) > static char * > setup_buffer(char *buffer, int addlen) > { > - size_t len; > + size_t len, old_len; > + char *new_buffer; > > setup_buffer_bufsiz = screen_width; > if (setup_buffer_bufsiz < SETUPBUFFER_MIN_SCREENWIDTH) > @@ -1355,13 +1356,18 @@ setup_buffer(char *buffer, int addlen) > setup_buffer_bufsiz = SETUPBUFFER_MIN_SCREENWIDTH; > } > > - free(buffer); > len = setup_buffer_bufsiz + addlen + SETUPBUFFER_REQUIRED_ADDBUFSIZ; > - buffer = calloc(len, sizeof(char)); > - if (buffer == NULL) > + new_buffer = calloc(len, sizeof(char)); > + if (new_buffer == NULL) > { > errx(4, "can't allocate sufficient memory"); > } > + if (buffer != NULL) > + { > + old_len = strlen(buffer); > + memcpy(new_buffer, buffer, old_len < len - 1 ? old_len : len - 1); > + free(buffer); > + } > > - return buffer; > + return new_buffer; > } Looks like realloc() hasn't been invented yet. realloc() wouldn't clear the new part of the buffer, so a memset() or at least setting the first byte in a new buffer (starting with buffer == NULL might be needed). The above has some bugs when the new buffer is smaller the old buffer: - when old_len < len - 1, the new buffer has no space for the old buffer including its NUL terminator, so the new buffer is left unterminated after blind truncation - when old_len == len - 1, the new buffer has no space for the NUL terminator, so the new buffer is left unterminated after not overrunning it by copying the NUL terminator - when old_len > len - 1, the new buffer is NUL terminated in an obfuscated way (calloc() has filled it with NULs and the memcpy() doesn't overwrite them all). Bruce