From owner-svn-src-all@freebsd.org Tue Sep 26 12:16:35 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 24B18E08FFF; Tue, 26 Sep 2017 12:16:35 +0000 (UTC) (envelope-from manu@bidouilliste.com) Received: from mail.blih.net (mail.blih.net [212.83.177.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.blih.net", Issuer "mail.blih.net" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 299F863938; Tue, 26 Sep 2017 12:16:33 +0000 (UTC) (envelope-from manu@bidouilliste.com) Received: from mail.blih.net (mail.blih.net [212.83.177.182]) by mail.blih.net (OpenSMTPD) with ESMTP id 6882f2e8; Tue, 26 Sep 2017 14:16:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=bidouilliste.com; h=date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; s=mail; bh=nt5RnFcvohjtPaF45IXMvRTJHzs=; b=fcCt/Y8oFr5lO2ax/6isTrqNW9JU eDwITGr+7EkXu1rwLrUAwARFwx5eU2g6F7ex8cQsOMByvMYlRAgY7H1ZdJ8YS/QL XVf6QzjdjAYcaDZ8D3kkqWeMBrWNtOlOkA9X4Gv3GgOoo+DaG2D+Y/5fll4yYWt+ asahlcR9R37QM64= DomainKey-Signature: a=rsa-sha1; c=nofws; d=bidouilliste.com; h=date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; q=dns; s= mail; b=JrZ5aZrclpxKZv51tK/DJ8MFHIyQ55YPGVNAJOdFCR620TWXOswAEz/U Rm7WXYWQffKIHoiUgxxfwwLw5ZjUR1VD9ijIjFVm3O5BWgCYkrRBRGRRBFwcRaD6 iSGYXWrS/CuAK25yOvL1+fIJddbeZL42Qk6nC+s0sRZOI75DqbA= Received: from arcadia (evadot.gandi.net [217.70.181.36]) by mail.blih.net (OpenSMTPD) with ESMTPSA id ef280b6b TLS version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO; Tue, 26 Sep 2017 14:16:30 +0200 (CEST) Date: Tue, 26 Sep 2017 14:16:30 +0200 From: Emmanuel Vadot To: Bruce Evans Cc: Konstantin Belousov , Emmanuel Vadot , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r324007 - head/usr.sbin/mountd Message-Id: <20170926141630.9bdf32c09ac3a71e33e08b68@bidouilliste.com> In-Reply-To: <20170926212215.T1299@besplex.bde.org> References: <201709260918.v8Q9II14056929@repo.freebsd.org> <20170926102457.GG2271@kib.kiev.ua> <20170926131135.73b1f216e9448d863f7b48a9@bidouilliste.com> <20170926212215.T1299@besplex.bde.org> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; amd64-portbld-freebsd12.0) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 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: Tue, 26 Sep 2017 12:16:35 -0000 On Tue, 26 Sep 2017 22:09:53 +1000 (EST) Bruce Evans wrote: > On Tue, 26 Sep 2017, Emmanuel Vadot wrote: > > > On Tue, 26 Sep 2017 13:24:57 +0300 > > Konstantin Belousov wrote: > > > >> On Tue, Sep 26, 2017 at 09:18:18AM +0000, Emmanuel Vadot wrote: > >>> @@ -1940,14 +1936,16 @@ add_expdir(struct dirlist **dpp, char *cp, int len) > >>> { > >>> struct dirlist *dp; > >>> > >>> - dp = (struct dirlist *)malloc(sizeof (struct dirlist) + len); > >>> + dp = (struct dirlist *)malloc(sizeof (struct dirlist)); > >> You might remove the unneeded cast as well. > > > > Right, done in 324012. Hi Bruce, > Er, mountd has many similar casts, not just the one fixed, and worse ones > like casting dp to caddr_t before passing it to free() (caddr_t is bogus, > and free() doesn't actually take it -- its prototype converts caddr_t to > void *, just like it would convert dp's type to void *)). Some of these > casts were needed in 1987 for unprototyped pre-StandardC code. caddr_t > was more needed in 1977 before void * existed (free() takes a char * arg > in K&R1). I've fixed this one in 324014 and will probably apply style(9) to the whole file at some point. > But the main hug near here is leaking memory for strdup()). The above > malloc() allocates 2 storage areas together (1 for the string at the > end), and seems to have a corresponding free(). Now there is an extra > separate storage error for the string and no separate free() for it. Yes, sorry for that, this is fixed in 324014 too. > This is not much more than a style bug too. mountd is a long-lived > daemon, so it should avoid leaking memory, but it would probably work > OK for a few thousand mounts with no free()s at all or a few million > with no frees of strings. But it tries to free() most things, so it > is a style bug to not try for some. > > Bruce Thanks, -- Emmanuel Vadot