Memory leak in libsmb/smbc_opendir() [reformatted]

[Richard Sharpe]

Hi,

Thanks for this.

I will try to apply this patch today.

Ong Kian Win wrote:

>Hi.
>
>smbc_opendir() would call cli_initialise() (through smbc_server()). However,
>cli_shutdown() would not be called at all. As such, there would be a memory
>leak.
>
>I suggest that smbc_closedir() call smbc_remove_unused_server().
>
>For depth-first traversals, calling smbc_opendir() would not result in
>additional connections being made, since the connection would have been
>established higher up in the file system. For smbc_opendir() and
>smbc_closedir() pairs which are *not nested*, however, there could
>potentially be a degradation in performance. smbc_closedir() would close the
>connection, requiring a new connection to be made every time.
>
>I am not sure if there is a good solution to this caveat. libsmb has
>abstracted away establishing a connection. Where you would normally do
>
>connect(share)
>	opendir
>	closedir
>	opendir
>	closedir
>disconnect(share)
>
>you would now do
>
>opendir(share)
>closedir (share)
>opendir
>closedir
>opendir
>closedir
>
>Regards,
>Kian Win
>
>Samba CVS HEAD, Feb 25 10:09 +0800
>
>--- libsmbclient.c.bkp.0	Wed Jan 30 14:08:19 2002
>+++ libsmbclient.c	Tue Feb 26 20:52:16 2002
>@@ -1508,6 +1508,36 @@
>
> }
>
>+/*
>+ * Routine to open a directory
>+ *
>+ * We want to allow:
>+ *
>+ * smb: which should list all the workgroups available
>+ * smb:workgroup
>+ * smb:workgroup//server
>+ * smb://server
>+ * smb://server/share
>+ * smb://<IP-addr> which should list shares on server
>+ * smb://<IP-addr>/share which should list files on share
>+ *
>+ */
>+
>+/*
>+ * Note that smbc_opendir("smb://server/share") and
>+ * smbc_opendir("smb://server/share/dir") would cache
>+ * the connection to the target share.
>+ *
>+ * smbc_closedir will remove that connection from
>+ * the cache immediately.
>+ *
>+ * This should not degrade performance for depth-first
>+ * traversals of shares. If you need breadth-first
>+ * traversals, you might want to do a dummy smbc_opendir,
>+ * and smbc_closedir only when you are done with the
>+ * entire share.
>+ */
>+
> int smbc_opendir(const char *fname)
> {
> 	fstring server, share, user, password, workgroup;
>@@ -1811,6 +1841,7 @@
> int smbc_closedir(int fd)
> {
> 	struct smbc_file *fe;
>+	struct smbc_server *srv;
>
> 	if (!smbc_initialized) {
>
>@@ -1837,14 +1868,11 @@
>
> 	smbc_remove_dir(fe); /* Clean it up */
>
>-	if (fe) {
>-
>-		SAFE_FREE(fe->fname);
>-		SAFE_FREE(fe);    /* Free the space too */
>-
>-	}
>-
>+	srv = fe->srv;
>+	SAFE_FREE(fe->fname);
>+	SAFE_FREE(fe);       /* Free the space too */
> 	smbc_file_table[fd - smbc_start_fd] = NULL;
>+	smbc_remove_unused_server(srv);
>
> 	return 0;
>
>
>
>

-- 
Richard Sharpe, rsharpe at ns.aus.com, rsharpe at samba.org, sharpe at ethereal.com
www.samba.org, www.ethereal.com, SAMS Teach Yourself Samba 
in 24 Hours, Special Edition, Using Samba