On Tue, 26 Sep 2017, Emmanuel Vadot wrote:

On Tue, 26 Sep 2017 13:24:57 +0300
Konstantin Belousov <kostik...@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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to