From owner-svn-src-all@freebsd.org Tue Sep 26 12:10:03 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 EC9CDE08C96; Tue, 26 Sep 2017 12:10:03 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id B52933FED; Tue, 26 Sep 2017 12:10:03 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 4428D42356C; Tue, 26 Sep 2017 22:09:54 +1000 (AEST) Date: Tue, 26 Sep 2017 22:09:53 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Emmanuel Vadot 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 In-Reply-To: <20170926131135.73b1f216e9448d863f7b48a9@bidouilliste.com> Message-ID: <20170926212215.T1299@besplex.bde.org> References: <201709260918.v8Q9II14056929@repo.freebsd.org> <20170926102457.GG2271@kib.kiev.ua> <20170926131135.73b1f216e9448d863f7b48a9@bidouilliste.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=LI0WeNe9 c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=pGLkceISAAAA:8 a=w7ArmIgtcycLYhWLe3YA:9 a=o3NbsaZe5tbfgi_4:21 a=9YDDRxIWfjNFlw7z:21 a=CjuIK1q_8ugA:10 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:10:04 -0000 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. 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). 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. 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