Date: Fri, 25 May 2018 23:23:59 +0800 From: Marcelo Araujo <araujobsdport@gmail.com> To: Brooks Davis <brooks@freebsd.org> Cc: Marcelo Araujo <araujo@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r334199 - head/usr.sbin/bhyve Message-ID: <CAOfEmZgV9yssn5v8ZpbkwL=rrifoD1Z=uRxe6a0KyM3mrXrSjQ@mail.gmail.com> In-Reply-To: <20180525151134.GB99063@spindle.one-eyed-alien.net> References: <201805250207.w4P275Pf060725@repo.freebsd.org> <20180525151134.GB99063@spindle.one-eyed-alien.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, May 25, 2018, 11:11 PM Brooks Davis <brooks@freebsd.org> wrote: > On Fri, May 25, 2018 at 02:07:05AM +0000, Marcelo Araujo wrote: > > Author: araujo > > Date: Fri May 25 02:07:05 2018 > > New Revision: 334199 > > URL: https://svnweb.freebsd.org/changeset/base/334199 > > > > Log: > > Fix a memory leak on topology_parse(). > > > > strdup(3) allocates memory for a copy of the string, does the copy and > > returns a pointer to it. If there is no sufficient memory NULL is > returned > > and the global errno is set to ENOMEM. > > We do a sanity check to see if it was possible to allocate enough > memory. > > > > Also as we allocate memory, we need to free this memory used. Or it > will > > going out of scope leaks the storage it points to. > > > > Reviewed by: rgrimes > > MFC after: 3 weeks. > > X-MFC: r332298 > > Sponsored by: iXsystems Inc. > > Differential Revision: https://reviews.freebsd.org/D15550 > > > > Modified: > > head/usr.sbin/bhyve/bhyverun.c > > > > Modified: head/usr.sbin/bhyve/bhyverun.c > > > ============================================================================== > > --- head/usr.sbin/bhyve/bhyverun.c Fri May 25 01:38:59 2018 > (r334198) > > +++ head/usr.sbin/bhyve/bhyverun.c Fri May 25 02:07:05 2018 > (r334199) > > @@ -193,6 +193,7 @@ topology_parse(const char *opt) > > c = 1, n = 1, s = 1, t = 1; > > ns = false, scts = false; > > str = strdup(opt); > > + assert(str != NULL); > > Using assert seems like an odd choice when you've already added a > failure path and the strsep will crash immediately if assert is elided. > Just to make a better point, I had the same discussion about assert(3) in another review, we don't do NDEBUG even for RELEASE. > > -- Brooks >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOfEmZgV9yssn5v8ZpbkwL=rrifoD1Z=uRxe6a0KyM3mrXrSjQ>