From owner-svn-src-all@freebsd.org Fri May 25 17:11:31 2018 Return-Path: Delivered-To: svn-src-all@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 1FD17EEE2A1 for ; Fri, 25 May 2018 17:11:31 +0000 (UTC) (envelope-from lists@eitanadler.com) Received: from mail-yw0-x242.google.com (mail-yw0-x242.google.com [IPv6:2607:f8b0:4002:c05::242]) (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 A2E4069DC9 for ; Fri, 25 May 2018 17:11:30 +0000 (UTC) (envelope-from lists@eitanadler.com) Received: by mail-yw0-x242.google.com with SMTP id p14-v6so1935846ywm.11 for ; Fri, 25 May 2018 10:11:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eitanadler.com; s=0xdeadbeef; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=PUVDImQdwzxn5XtVjD5zYisjHMfcjDNl9V14fxCelaI=; b=DXYFLa2xotZ+6e53hPFlLtANz0j/xH7h9Y5MT0Xzcb1vOoaxES4R7YKOQs8rJQgKTd Sb+S1ZqYW4BC/TtWvqjvIkhqYgZkXiLC7FNN2EhX2aerFMDGH5iNUPNgQwb9enYpxZex WSsVozfwFNVidAYVXrW8BbPXyEODJT3bok3yU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=PUVDImQdwzxn5XtVjD5zYisjHMfcjDNl9V14fxCelaI=; b=KAawQLmccXVZ4WqzWFDVP0EsQR+JrfaUq27zW/xZarLse0DkedEm0dX8OF2rf+bOXV g56mifNAXeG+en9G7ObaGay5AjupG046jNWT3EuDRnpVoH65t6+QD3b8XXCNq58kYUir tvLrVLwGFqs25CUra9Q8bYS3l0EBRBgjJeQpXxmvg6dfQ5mJsngnpIv3cV0E/l6Ml+mn fDHbrJouxr7rbTjEwdhiOBzfFB+RZZBaht6Obq3eBIOwCcsMN6Pw38LpB2m7jRfL1Wba pXSfheOjFffOf4FhNnphhrXp/ICnPWNwKEuxr/lQMmDFJq5cV2ShmlPeE6hj59sYAAvh iUrg== X-Gm-Message-State: ALKqPwdLkzc5YH+BQnS9P/fxnr5xpJdEVjqdyGc0fA/D9Rzb+41yEm9c BnL60qUWqJWFxi0TYrS3dbFRRfODUJcwGC3AlHJWMg== X-Google-Smtp-Source: AB8JxZornnOqmUh6KA2bVySB7YYmVc0Xqx8r0BSWls+RiCBCvYMd/CZrxhTcdQgaRHbow0Hcc6+GW9q9voGgyFxYBes= X-Received: by 2002:a81:5fc4:: with SMTP id t187-v6mr1859106ywb.19.1527268289941; Fri, 25 May 2018 10:11:29 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a25:c709:0:0:0:0:0 with HTTP; Fri, 25 May 2018 10:10:59 -0700 (PDT) In-Reply-To: References: <201805250207.w4P275Pf060725@repo.freebsd.org> <20180525151134.GB99063@spindle.one-eyed-alien.net> From: Eitan Adler Date: Fri, 25 May 2018 10:10:59 -0700 Message-ID: Subject: Re: svn commit: r334199 - head/usr.sbin/bhyve To: araujo@freebsd.org Cc: Brooks Davis , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 May 2018 17:11:31 -0000 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. -- Eitan Adler