svn commit: r324007 - head/usr.sbin/mountd
Bruce Evans
brde at optusnet.com.au
Tue Sep 26 12:10:04 UTC 2017
On Tue, 26 Sep 2017, Emmanuel Vadot wrote:
> On Tue, 26 Sep 2017 13:24:57 +0300
> Konstantin Belousov <kostikbel at gmail.com> 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
More information about the svn-src-all
mailing list