Date: Thu, 4 Apr 2019 00:16:58 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Dimitry Andric <dim@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r345807 - head/usr.bin/top Message-ID: <20190403234558.X1970@besplex.bde.org> In-Reply-To: <201904021801.x32I1sxX019439@repo.freebsd.org> References: <201904021801.x32I1sxX019439@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <james.wright@jigsawdezign.com> > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190403234558.X1970>