From owner-svn-src-head@freebsd.org Fri May 25 17:21:47 2018 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 07F6AEEE74D; Fri, 25 May 2018 17:21:47 +0000 (UTC) (envelope-from araujobsdport@gmail.com) Received: from mail-wm0-x22d.google.com (mail-wm0-x22d.google.com [IPv6:2a00:1450:400c:c09::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 790B66A76C; Fri, 25 May 2018 17:21:46 +0000 (UTC) (envelope-from araujobsdport@gmail.com) Received: by mail-wm0-x22d.google.com with SMTP id m129-v6so16436488wmb.3; Fri, 25 May 2018 10:21:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:reply-to:from:date:message-id :subject:to:cc; bh=rPl05NE36n0bcoV8NDLXztzKtryVOBPus2I8gjInUIs=; b=ZmzHVfvUBtPeCjobXlShY8AwEvSVCtaCwyWQjfWPU9V3JpCI/RRskyUdBlUS4JP3Fa 2VUA/BDH8ZlsMg3glSAio8ty4mQsVWkD+PjlXzaEDneZ8Wd1AbIq88eCHCDqIhxFcUVz syXwUfgP3imVROmEfWz9rtXf8v2IfrztVcgkOHZosAsZ4ld183zT1aGLCsog41IlRjDx +YN11uGObn9LN0fVP1XAJjp7Wyo69wT13OfjpR3fo3v02sXBE8Qb79wyUf/Jr10CMvDd W28vEhU244uUruEUcMT/LmX9OO+OnSKdRFu1Ml8apriaPhCJVWNNuYH9+RZ9jeb3fY7u EJ+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:reply-to :from:date:message-id:subject:to:cc; bh=rPl05NE36n0bcoV8NDLXztzKtryVOBPus2I8gjInUIs=; b=P5RKe2RxBE7c3LV7ttkJ9ByDpx85/tudnto9YsIQx0c5uiEGqJZPGHPzYc51ok+N6K QAA+xqsxDmC1O8V3zf6dLISHVsOlhnm9miqZg5QeQhmdDgbZWtyKxBsPU0GONcoegJoC VE1Oi7Hh6aiwKtXFWBmu1W03OD/m/Ar+xjt2LKsH7FxqJDJekoNdLQPcHfjupdZC6Mfq WKKzGmBsYYj1M6lVFz/sMEprCZlleUnmMaJCesNDH3B7MnxLDZrUrpz2rJUhv0kBt6i4 LhkG2GgeO2YZgrmqbYW3OJIkJ+CmaKEVwSOKfFsKfp8q+p9DwLfGZcjr9ouCD/xPQU1C l9jg== X-Gm-Message-State: ALKqPwdJ1Rbx13X6t8ApuQwekOHd9AVrAufumRf1+sxjxeC2Jm/x37A6 +mmOXz30oOsrRCYJs8jgZYic+Er1ZplvG6Xz1mQ= X-Google-Smtp-Source: AB8JxZrb0+gufUHUOMBk4P107Xw+fO4d7cY6Efkp8yOMrK626ILtGhfoXx7lFLH3EBkmmn3ERD8g+hIDIFdWmrQ9OYs= X-Received: by 2002:a2e:80c1:: with SMTP id r1-v6mr2322684ljg.85.1527268905441; Fri, 25 May 2018 10:21:45 -0700 (PDT) MIME-Version: 1.0 References: <201805250207.w4P275Pf060725@repo.freebsd.org> <20180525151134.GB99063@spindle.one-eyed-alien.net> In-Reply-To: Reply-To: araujo@freebsd.org From: Marcelo Araujo Date: Sat, 26 May 2018 01:21:33 +0800 Message-ID: Subject: Re: svn commit: r334199 - head/usr.sbin/bhyve To: Eitan Adler Cc: Marcelo Araujo , Brooks Davis , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.26 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.26 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: Fri, 25 May 2018 17:21:47 -0000 On Sat, May 26, 2018, 1:11 AM Eitan Adler wrote: > On 25 May 2018 at 08:23, Marcelo Araujo wrote: > > > > > > On Fri, May 25, 2018, 11:11 PM Brooks Davis 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. > Again, in this case we are using it all over the place and we must replace it. Also we should document it in somewhere perhaps in the assert(3) otherwise myself and others will keep using it. If you use find, not only myself is using it to check strdup! So what is the suggestion to handle assert(3)? Deprecated it? > > > > > -- > Eitan Adler >