memory leak -- comments?

[Jeremy Allison]

On Thu, Dec 15, 2005 at 06:39:01PM -0500, derrell at samba.org wrote:
> Hi Jeremy,
> 
> I received the attached report, including valgrind output.  It looks to me as
> if cli_receive_trans() allocates 'param' and 'data', and then, back in
> cli_list_new(), if one of these three conditions is true, we'll have a memory
> leak:
> 
>   if (cli_is_error_cli(cli) || !rdata || !rparam)
> 
> or
> 
>   if (ff_searchcount == 0)
> 
> or
>   if (!tdl)      /* malloc failed)
> 
> Of these, the one likely to occur is ff_searchcount == 0.  I suspect that
> was the cause of this report.  Agree?  Each of these cases should be
> SAFE_FREE()ing rdata and rparam, right?

Here's the patch.

Jeremy.
-------------- next part --------------
Index: libsmb/clilist.c
===================================================================
--- libsmb/clilist.c	(revision 12270)
+++ libsmb/clilist.c	(working copy)
@@ -271,6 +271,10 @@
 			   it gives ERRSRV/ERRerror temprarily */
 			uint8 eclass;
 			uint32 ecode;
+
+			SAFE_FREE(rdata);
+			SAFE_FREE(rparam);
+
 			cli_dos_error(cli, &eclass, &ecode);
 			if (eclass != ERRSRV || ecode != ERRerror)
 				break;
@@ -278,8 +282,11 @@
 			continue;
 		}
 
-                if (cli_is_error(cli) || !rdata || !rparam) 
+                if (cli_is_error(cli) || !rdata || !rparam) {
+			SAFE_FREE(rdata);
+			SAFE_FREE(rparam);
 			break;
+		}
 
 		if (total_received == -1)
 			total_received = 0;
@@ -297,8 +304,11 @@
 			ff_lastname = SVAL(p,6);
 		}
 
-		if (ff_searchcount == 0) 
+		if (ff_searchcount == 0) {
+			SAFE_FREE(rdata);
+			SAFE_FREE(rparam);
 			break;
+		}
 
 		/* point to the data bytes */
 		p = rdata;
@@ -332,6 +342,8 @@
 
 		if (!tdl) {
 			DEBUG(0,("cli_list_new: Failed to expand dirlist\n"));
+			SAFE_FREE(rdata);
+			SAFE_FREE(rparam);
 			break;
 		} else {
 			dirlist = tdl;