Date: Fri, 25 May 2018 10:10:59 -0700 From: Eitan Adler <lists@eitanadler.com> To: araujo@freebsd.org Cc: Brooks Davis <brooks@freebsd.org>, src-committers <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: <CAF6rxgm32%2B_XazDvbtyFChPigxVB0HQ30r3=CvN65ko=zHq0yA@mail.gmail.com> In-Reply-To: <CAOfEmZgV9yssn5v8ZpbkwL=rrifoD1Z=uRxe6a0KyM3mrXrSjQ@mail.gmail.com> References: <201805250207.w4P275Pf060725@repo.freebsd.org> <20180525151134.GB99063@spindle.one-eyed-alien.net> <CAOfEmZgV9yssn5v8ZpbkwL=rrifoD1Z=uRxe6a0KyM3mrXrSjQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 25 May 2018 at 08:23, Marcelo Araujo <araujobsdport@gmail.com> wrote: > > > 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. IMHO we only use assert for asserting things ought to never be false except in buggy code. Using assert for handling is poor practice. -- Eitan Adler
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF6rxgm32%2B_XazDvbtyFChPigxVB0HQ30r3=CvN65ko=zHq0yA>