Subject: Re: Memory leak in nfsrv_readdir
To: None <"Undisclosed.Recipients">
From: Mark Davies <mark@mcs.vuw.ac.nz>
List: tech-net
Date: 07/21/2006 09:52:30
On Friday 21 July 2006 03:07, Christos Zoulas wrote:

> You don't need the caddr_t cast, and isn't it more efficient to free
> the cookies before the statement 'goto again' unconditionally, and without
> setting cookies to NULL? Yes, it is not as safe, but there is only
> one goto again in each function (readdir/readdirplus)...

Sure.  I suggested this way to be consistent with how FreeBSD (and OpenBSD) do 
it.  And yes you don't need the cast but I'll note that the existing free's 
of cookies in those routines all have the cast.

So is my previous patch preferred, to keep the code in line with the other 
BSD's or should we go with the below?

Index: nfs_serv.c
===================================================================
RCS file: /src/cvs/netbsd/src/sys/nfs/nfs_serv.c,v
retrieving revision 1.98
diff -u -r1.98 nfs_serv.c
--- nfs_serv.c	6 Oct 2005 10:23:01 -0000	1.98
+++ nfs_serv.c	20 Jul 2006 21:46:22 -0000
@@ -2734,6 +2734,7 @@
 	if (cpos >= cend || ncookies == 0) {
 		toff = off;
 		siz = fullsiz;
+		free(cookies, M_TEMP);
 		goto again;
 	}
 
@@ -3005,6 +3006,7 @@
 	if (cpos >= cend || ncookies == 0) {
 		toff = off;
 		siz = fullsiz;
+		free(cookies, M_TEMP);
 		goto again;
 	}
 

cheers
mark