smbd: Simplify the flow in close_file_free()

We are no longer called on base_fsp's in SHUTDOWN_CLOSE. That
simplifies the logic in the common case, we now have a linear flow for
the very often-called close_file()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14975

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
(cherry picked from commit 93fe9c83145d31ea11a9cd25049ac527ad4a000d)

1 files changed, 19 insertions, 50 deletions

diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 9160266c6fb..9e7f89a304f 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -1484,8 +1484,6 @@ NTSTATUS close_file_free(struct smb_request *req,
 {
 	NTSTATUS status;
 	struct files_struct *fsp = *_fsp;
-	struct files_struct *base_fsp = fsp->base_fsp;
-	bool close_base_fsp = false;
 
 	/*
 	 * This fsp can never be an internal dirfsp. They must
@@ -1493,44 +1491,10 @@ NTSTATUS close_file_free(struct smb_request *req,
 	 */
 	SMB_ASSERT(!fsp->fsp_flags.is_dirfsp);
 
-	if (fsp->stream_fsp != NULL) {
-		/*
-		 * fsp is the base for a stream.
-		 *
-		 * We're called with SHUTDOWN_CLOSE from files.c which walks the
-		 * complete list of files.
-		 *
-		 * We need to wait until the stream is closed.
-		 */
-		SMB_ASSERT(close_type == SHUTDOWN_CLOSE);
-		return NT_STATUS_OK;
-	}
-
-	if (base_fsp != NULL) {
-		/*
-		 * We need to remove the link in order to
-		 * recurse for the base fsp below.
-		 */
-		SMB_ASSERT(base_fsp->base_fsp == NULL);
-		SMB_ASSERT(base_fsp->stream_fsp == fsp);
-		base_fsp->stream_fsp = NULL;
-
-		if (close_type == SHUTDOWN_CLOSE) {
-			/*
-			 * We're called with SHUTDOWN_CLOSE from files.c
-			 * which walks the complete list of files.
-			 *
-			 * We may need to defer the SHUTDOWN_CLOSE
-			 * if it's the next in the linked list.
-			 *
-			 * So we only close if the base is *not* the
-			 * next in the list.
-			 */
-			close_base_fsp = (fsp->next != base_fsp);
-		} else {
-			close_base_fsp = true;
-		}
-	}
+	/*
+	 * Never call directly on a base fsp
+	 */
+	SMB_ASSERT(fsp->stream_fsp == NULL);
 
 	if (fsp->fake_file_handle != NULL) {
 		status = close_fake_file(req, fsp);
@@ -1557,23 +1521,28 @@ NTSTATUS close_file_free(struct smb_request *req,
 		status = close_normal_file(req, fsp, close_type);
 	}
 
-	file_free(req, fsp);
+	if (fsp->base_fsp != NULL) {
+		/*
+		 * fsp was a stream, its base_fsp can't be a stream
+		 * as well
+		 */
+		SMB_ASSERT(fsp->base_fsp->base_fsp == NULL);
 
-	if (close_base_fsp) {
+		/*
+		 * There's a 1:1 relationship between fsp and a base_fsp
+		 */
+		SMB_ASSERT(fsp->base_fsp->stream_fsp == fsp);
 
 		/*
-		 * fsp was a stream, the base fsp can't be a stream as well
-		 *
-		 * For SHUTDOWN_CLOSE this is not possible here
-		 * (if the base_fsp was the next in the linked list), because
-		 * SHUTDOWN_CLOSE only happens from files.c which walks the
-		 * complete list of files. If we mess with more than one fsp
-		 * those loops will become confused.
+		 * Make base_fsp look standalone now
 		 */
+		fsp->base_fsp->stream_fsp = NULL;
 
-		close_file_free(req, &base_fsp, close_type);
+		close_file_free(req, &fsp->base_fsp, close_type);
 	}
 
+	file_free(req, fsp);
+
 	*_fsp = NULL;
 
 	return status;