Re: svn commit: r336031 - head/usr.bin/top
In message <20180706135634.gp5...@kib.kiev.ua>, Konstantin Belousov writes: > On Fri, Jul 06, 2018 at 01:22:44PM +, Sean Bruno wrote: > > Author: sbruno > > Date: Fri Jul 6 13:22:44 2018 > > New Revision: 336031 > > URL: https://svnweb.freebsd.org/changeset/base/336031 > > > > Log: > > r336028 changed next_msg to a char * from char [] of fixed size. Change > > 2nd argument of vsnprintf() to get the strlen of next_msg so that the > > appropriate size is used. > > > > Found with gcc. > > > > /usr.bin/top/display.c: In function 'new_message': > > /usr.bin/top/display.c:963:31: error: > > argument to 'sizeof' in 'vsnprintf' call is the same expression as the > > destination; did you mean to provide an explicit length? > > [-Werror=sizeof-pointer-memaccess] > >vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); > > > > Reviewed by: daichi > > > > Modified: > > head/usr.bin/top/display.c > > > > Modified: head/usr.bin/top/display.c > > === > === > > --- head/usr.bin/top/display.c Fri Jul 6 12:44:48 2018(r33603 > 0) > > +++ head/usr.bin/top/display.c Fri Jul 6 13:22:44 2018(r33603 > 1) > > @@ -960,7 +960,7 @@ new_message(int type, const char *msgfmt, ...) > > va_start(args, msgfmt); > > > > /* first, format the message */ > > -vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); > > +vsnprintf(next_msg, strlen(next_msg), msgfmt, args); > I highly suspect that this strlen() call returns zero, always. Yes. OTOH sizeof(next_msg) won't return much useful either, as it returns the size of the pointer. > > > > > va_end(args); > > > -- Cheers, Cy Schubert FreeBSD UNIX: Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r336031 - head/usr.bin/top
On 07/06/18 08:49, Ian Lepore wrote: > On Fri, 2018-07-06 at 13:22 +, Sean Bruno wrote: >> Author: sbruno >> Date: Fri Jul 6 13:22:44 2018 >> New Revision: 336031 >> URL: https://svnweb.freebsd.org/changeset/base/336031 >> >> Log: >> r336028 changed next_msg to a char * from char [] of fixed size. Change >> 2nd argument of vsnprintf() to get the strlen of next_msg so that the >> appropriate size is used. >> >> Found with gcc. >> >> /usr.bin/top/display.c: In function 'new_message': >> /usr.bin/top/display.c:963:31: error: >> argument to 'sizeof' in 'vsnprintf' call is the same expression as the >> destination; did you mean to provide an explicit length? >> [-Werror=sizeof-pointer-memaccess] >> vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); >> >> Reviewed by: daichi >> >> Modified: >> head/usr.bin/top/display.c >> >> Modified: head/usr.bin/top/display.c >> == >> --- head/usr.bin/top/display.c Fri Jul 6 12:44:48 2018 >> (r336030) >> +++ head/usr.bin/top/display.c Fri Jul 6 13:22:44 2018 >> (r336031) >> @@ -960,7 +960,7 @@ new_message(int type, const char *msgfmt, ...) >> va_start(args, msgfmt); >> >> /* first, format the message */ >> -vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); >> +vsnprintf(next_msg, strlen(next_msg), msgfmt, args); >> >> va_end(args); >> >> > > This fix is incorrect. The original commit that changed next_msg to a > pointer is probably flawed enough to revert and redevelop rather than > try a rolling set of bandaid fixes. > > Whenever setup_buffer() creates a new buffer it will need to store the > size it allocated for use in this vsnprintf() call (and maybe other > places that write directly into next_msg without calling setup_buffer > to reallocate it first, I didn't look). The setup_buffer_bufsiz > variable isn't quite right as-is, because it doesn't include the > 'addlen' value passed to setup_buffer(). > > -- Ian > > Yeah, this isn't going well. I've been poking around in it and I can't see an quick way to do this correctly. sean signature.asc Description: OpenPGP digital signature
Re: svn commit: r336031 - head/usr.bin/top
On Fri, 2018-07-06 at 13:22 +, Sean Bruno wrote: > Author: sbruno > Date: Fri Jul 6 13:22:44 2018 > New Revision: 336031 > URL: https://svnweb.freebsd.org/changeset/base/336031 > > Log: > r336028 changed next_msg to a char * from char [] of fixed size. Change > 2nd argument of vsnprintf() to get the strlen of next_msg so that the > appropriate size is used. > > Found with gcc. > > /usr.bin/top/display.c: In function 'new_message': > /usr.bin/top/display.c:963:31: error: > argument to 'sizeof' in 'vsnprintf' call is the same expression as the > destination; did you mean to provide an explicit length? > [-Werror=sizeof-pointer-memaccess] > vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); > > Reviewed by:daichi > > Modified: > head/usr.bin/top/display.c > > Modified: head/usr.bin/top/display.c > == > --- head/usr.bin/top/display.cFri Jul 6 12:44:48 2018 > (r336030) > +++ head/usr.bin/top/display.cFri Jul 6 13:22:44 2018 > (r336031) > @@ -960,7 +960,7 @@ new_message(int type, const char *msgfmt, ...) > va_start(args, msgfmt); > > /* first, format the message */ > -vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); > +vsnprintf(next_msg, strlen(next_msg), msgfmt, args); > > va_end(args); > > This fix is incorrect. The original commit that changed next_msg to a pointer is probably flawed enough to revert and redevelop rather than try a rolling set of bandaid fixes. Whenever setup_buffer() creates a new buffer it will need to store the size it allocated for use in this vsnprintf() call (and maybe other places that write directly into next_msg without calling setup_buffer to reallocate it first, I didn't look). The setup_buffer_bufsiz variable isn't quite right as-is, because it doesn't include the 'addlen' value passed to setup_buffer(). -- Ian ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r336031 - head/usr.bin/top
On Fri, Jul 06, 2018 at 01:22:44PM +, Sean Bruno wrote: > Author: sbruno > Date: Fri Jul 6 13:22:44 2018 > New Revision: 336031 > URL: https://svnweb.freebsd.org/changeset/base/336031 > > Log: > r336028 changed next_msg to a char * from char [] of fixed size. Change > 2nd argument of vsnprintf() to get the strlen of next_msg so that the > appropriate size is used. > > Found with gcc. > > /usr.bin/top/display.c: In function 'new_message': > /usr.bin/top/display.c:963:31: error: > argument to 'sizeof' in 'vsnprintf' call is the same expression as the > destination; did you mean to provide an explicit length? > [-Werror=sizeof-pointer-memaccess] >vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); > > Reviewed by:daichi > > Modified: > head/usr.bin/top/display.c > > Modified: head/usr.bin/top/display.c > == > --- head/usr.bin/top/display.cFri Jul 6 12:44:48 2018 > (r336030) > +++ head/usr.bin/top/display.cFri Jul 6 13:22:44 2018 > (r336031) > @@ -960,7 +960,7 @@ new_message(int type, const char *msgfmt, ...) > va_start(args, msgfmt); > > /* first, format the message */ > -vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); > +vsnprintf(next_msg, strlen(next_msg), msgfmt, args); I highly suspect that this strlen() call returns zero, always. > > va_end(args); > ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r336031 - head/usr.bin/top
Author: sbruno Date: Fri Jul 6 13:22:44 2018 New Revision: 336031 URL: https://svnweb.freebsd.org/changeset/base/336031 Log: r336028 changed next_msg to a char * from char [] of fixed size. Change 2nd argument of vsnprintf() to get the strlen of next_msg so that the appropriate size is used. Found with gcc. /usr.bin/top/display.c: In function 'new_message': /usr.bin/top/display.c:963:31: error: argument to 'sizeof' in 'vsnprintf' call is the same expression as the destination; did you mean to provide an explicit length? [-Werror=sizeof-pointer-memaccess] vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); Reviewed by: daichi Modified: head/usr.bin/top/display.c Modified: head/usr.bin/top/display.c == --- head/usr.bin/top/display.c Fri Jul 6 12:44:48 2018(r336030) +++ head/usr.bin/top/display.c Fri Jul 6 13:22:44 2018(r336031) @@ -960,7 +960,7 @@ new_message(int type, const char *msgfmt, ...) va_start(args, msgfmt); /* first, format the message */ -vsnprintf(next_msg, sizeof(next_msg), msgfmt, args); +vsnprintf(next_msg, strlen(next_msg), msgfmt, args); va_end(args); ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
