From: Jeremy Allison <jra@samba.org>
Date: Wed, 17 Jul 2019 16:11:25 +0000 (-0700)
Subject: s3: smbd: Move the setting of the destructor of struct smb_Dir * up two levels.
X-Git-Tag: tdb-1.4.2~500
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d1441089722b7985df417494f6542e6560e99330;p=thirdparty%2Fsamba.git

s3: smbd: Move the setting of the destructor of struct smb_Dir * up two levels.

Previously, open_dir_safely() called OpenDir_internal() which
set the destructor.

Move setting the destructor into the callers of open_dir_safely()
as this will allow us to have different destructors for handle-based
calls.

The reason this is important is that I have a follow up patchset
that depends on this which makes all client directory enumerations
handle-based, calling OpenDir_fsp() only, and so the destructor there
will take care of the fsp back pointer.

Trying to keep a common destructor for handle-based and non-handle
based calls broke my brain when trying to separate the handle-based
calls from the non-handle based ones in my later patchset.

NB. The change in OpenDir_fsp() isn't a logic change as instead
of doing an early return from a function that sets the destructor,
we now fallthrough to setting the destructor then return, which
is identical.

Eventually the whole codepath using the fallback for non-handle
opens inside dptr_create() will go away and this simplifies the
code immensely. Some short term pain for long-term gain :-).

Added doxygen documentation as requested.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
---

diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index bf829e75286..f1065e3a75f 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -90,6 +90,7 @@ static struct smb_Dir *open_dir_safely(TALLOC_CTX *ctx,
 					const struct smb_filename *smb_dname,
 					const char *wcard,
 					uint32_t attr);
+static int smb_Dir_destructor(struct smb_Dir *dirp);
 
 #define INVALID_DPTR_KEY (-3)
 
@@ -342,6 +343,10 @@ static struct smb_Dir *open_dir_with_privilege(connection_struct *conn,
 
 	dir_hnd = open_dir_safely(NULL, conn, smb_fname_cwd, wcard, attr);
 
+	if (dir_hnd != NULL) {
+		talloc_set_destructor(dir_hnd, smb_Dir_destructor);
+	}
+
   out:
 
 	vfs_ChDir(conn, saved_dir_fname);
@@ -442,7 +447,13 @@ NTSTATUS dptr_create(connection_struct *conn,
 					smb_dname_cp,
 					wcard,
 					attr);
+			if (dir_hnd != NULL) {
+				talloc_set_destructor(dir_hnd,
+					smb_Dir_destructor);
+			}
 		}
+
+
 		TALLOC_FREE(smb_dname_cp);
 	}
 
@@ -1559,8 +1570,6 @@ static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx,
 		dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn));
 	}
 
-	talloc_set_destructor(dirp, smb_Dir_destructor);
-
 	return dirp;
 
   fail:
@@ -1568,9 +1577,36 @@ static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx,
 	return NULL;
 }
 
-/****************************************************************************
- Open a directory handle by pathname, ensuring it's under the share path.
-****************************************************************************/
+/**
+ * @brief Open a directory handle by pathname, ensuring it's under the share path.
+ *
+ * First stores the $cwd, then changes directory to the passed in pathname
+ * uses check_name() to ensure this is under the connection struct share path,
+ * then operates on a pathname of "." to ensure we're in the same place.
+ *
+ * The returned struct smb_Dir * should have a talloc destrctor added to
+ * ensure that when the struct is freed the internal POSIX DIR * pointer
+ * is closed.
+ *
+ * @code
+ *
+ * static int sample_smb_Dir_destructor(struct smb_Dir *dirp)
+ * {
+ *     SMB_VFS_CLOSEDIR(dirp->conn,dirp->dir);
+ * }
+ * ..
+ *     struct smb_Dir *dir_hnd = open_dir_safely(mem_ctx,
+ *                              conn,
+ *                              smb_dname,
+ *                              mask,
+ *                              attr);
+ *      if (dir_hnd == NULL) {
+ *              return NULL;
+ *      }
+ *      talloc_set_destructor(dir_hnd, smb_Dir_destructor);
+ * ..
+ * @endcode
+ */
 
 static struct smb_Dir *open_dir_safely(TALLOC_CTX *ctx,
 					connection_struct *conn,
@@ -1627,6 +1663,7 @@ static struct smb_Dir *open_dir_safely(TALLOC_CTX *ctx,
 	dir_hnd->dir_smb_fname = cp_smb_filename(dir_hnd, smb_dname);
 	if (!dir_hnd->dir_smb_fname) {
 		TALLOC_FREE(dir_hnd);
+		SMB_VFS_CLOSEDIR(conn, dir_hnd->dir);
 		errno = ENOMEM;
 	}
 
@@ -1642,11 +1679,16 @@ struct smb_Dir *OpenDir(TALLOC_CTX *mem_ctx, connection_struct *conn,
 			const char *mask,
 			uint32_t attr)
 {
-	return open_dir_safely(mem_ctx,
+	struct smb_Dir *dir_hnd = open_dir_safely(mem_ctx,
 				conn,
 				smb_dname,
 				mask,
 				attr);
+	if (dir_hnd == NULL) {
+		return NULL;
+	}
+	talloc_set_destructor(dir_hnd, smb_Dir_destructor);
+	return dir_hnd;
 }
 
 /*******************************************************************
@@ -1708,11 +1750,15 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn,
 	if (dirp->dir == NULL) {
 		/* FDOPENDIR is not supported. Use OPENDIR instead. */
 		TALLOC_FREE(dirp);
-		return open_dir_safely(mem_ctx,
+		dirp = open_dir_safely(mem_ctx,
 					conn,
 					fsp->fsp_name,
 					mask,
 					attr);
+		if (dirp == NULL) {
+			errno = ENOMEM;
+			goto fail;
+		}
 	}
 
 	talloc_set_destructor(dirp, smb_Dir_destructor);