From: Jeremy Allison <jra@samba.org>
Date: Tue, 11 May 2021 18:41:42 +0000 (-0700)
Subject: s3: smbd: Remove a STAT/LSTAT call on the parent pathname in a hot code path.
X-Git-Tag: tevent-0.11.0~905
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=33f45491f8de52c3702c6f9e542261641e011c2e;p=thirdparty%2Fsamba.git

s3: smbd: Remove a STAT/LSTAT call on the parent pathname in a hot code path.

This optimization uses the stored errno result from check_parent_exists()
which already did a STAT/LSTAT if needed.

Best viewed with 'git show -b'.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Richard Sharpe <realrichardsharpe@gmail.com>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Tue May 11 22:52:58 UTC 2021 on sn-devel-184
---

diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 6181afce898..44a58a20c1f 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -1174,6 +1174,8 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx,
 		  state->smb_fname->base_name, state->dirpath, state->name);
 
 	if (!state->name_has_wildcard) {
+		int parent_stat_errno = 0;
+
 		/*
 		 * stat the name - if it exists then we can add the stream back (if
 		 * there was one) and be done!
@@ -1218,7 +1220,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx,
 						state->smb_fname,
 						&state->dirpath,
 						&state->name,
-						NULL);
+						&parent_stat_errno);
 			errno = saved_errno;
 			if (!NT_STATUS_IS_OK(status)) {
 				goto fail;
@@ -1252,29 +1254,30 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx,
 				/*
 				 * Was it a missing last component ?
 				 * or a missing intermediate component ?
+				 *
+				 * Optimization.
+				 *
+				 * For this code path we can guarantee that
+				 * we have gone through check_parent_exists()
+				 * and it returned NT_STATUS_OK.
+				 *
+				 * Either there was no parent component (".")
+				 * parent_stat_errno == 0 and we have a missing
+				 * last component here.
+				 *
+				 * OR check_parent_exists() called STAT/LSTAT
+				 * and if it failed parent_stat_errno has been
+				 * set telling us if the parent existed or not.
+				 *
+				 * Either way we can avoid another STAT/LSTAT
+				 * system call on the parent here.
 				 */
-				struct smb_filename *parent_fname = NULL;
-				struct smb_filename *base_fname = NULL;
-				bool ok;
-
-				ok = parent_smb_fname(state->mem_ctx,
-						      state->smb_fname,
-						      &parent_fname,
-						      &base_fname);
-				if (!ok) {
-					status = NT_STATUS_NO_MEMORY;
+				if (parent_stat_errno == ENOTDIR ||
+						parent_stat_errno == ENOENT ||
+						parent_stat_errno == ELOOP) {
+					status = NT_STATUS_OBJECT_PATH_NOT_FOUND;
 					goto fail;
 				}
-				ret = vfs_stat(state->conn, parent_fname);
-				TALLOC_FREE(parent_fname);
-				if (ret == -1) {
-					if (errno == ENOTDIR ||
-							errno == ENOENT ||
-							errno == ELOOP) {
-						status = NT_STATUS_OBJECT_PATH_NOT_FOUND;
-						goto fail;
-					}
-				}
 
 				/*
 				 * Missing last component is ok - new file.