[PATCH] Remove lp_posix_pathname() from synthetic_smb_fname_split()

Jeremy Allison jra at samba.org
Thu Mar 10 05:09:40 UTC 2016


OK, 6 patch set here that removes
one of the core pain-points of
lp_posix_pathnames() - its use
within synthetic_smb_fname_split().

It also refactors some horrible
code (I wrote many years ago) in
rename_internals_fsp() that
abused the struct smb_filename
struct for convenience, when
all I really should have used
was 2 char * pointers...

Patches in order:

1). Adds split_stream_filename()
function. Will be used in 2 places
in patches to follow.

2). Rewrites internals of synthetic_smb_fname_split()
to use the recently added split_stream_filename().
Code is much clearer.

3). Remove const SMB_STRUCT_STAT * parameter
from synthetic_smb_fname_split(). Only one
caller uses this, and is much clearer to
handle externally.

4). Remove lp_posix_pathnames() from
synthetic_smb_fname_split(). Pass as
a parameter instead.

This adds lp_posix_pathnames() rather
than removing them, but mostly the places
added are 'leaf' code - right outside
of the smbd code (mostly in the test
funtions used by vfstest where this
can eventually be replaced with an
environment variable lookup if we
ever need it). The one place inside
smbd where we add this will be removed
in patch #6.

5). Simplify logic inside rename_internals_fsp(),
part 1. Use standard parent_dirname() instead
of hand-crafted code that does the same thing.

6). Simplify logic inside rename_internals_fsp(),
part 2. Uses split_stream_filename() instead
of synthetic_smb_fname_split() to clean up
the logic and make is clearer. Removes
lp_posix_pathnames() added in patch #4.

Passes local make test.

Please review and push if happy.

We're well on the way to having
posix paths be a request-specific
variable inside smbd !

Cheers,

	Jeremy.
-------------- next part --------------
>From f7c40a5bdb6fe043f8cfd68e0b54b968ffe1ce06 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 9 Mar 2016 14:56:49 -0800
Subject: [PATCH 1/6] s3:lib. Add split_stream_filename() Not yet used.

Will replace internals of synthetic_smb_fname_split().

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/proto.h     |  4 ++++
 source3/lib/filename_util.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 0c15f96..b1ac7b0 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -1196,6 +1196,10 @@ bool is_ntfs_stream_smb_fname(const struct smb_filename *smb_fname);
 bool is_ntfs_default_stream_smb_fname(const struct smb_filename *smb_fname);
 bool is_invalid_windows_ea_name(const char *name);
 bool ea_list_has_invalid_name(struct ea_list *ea_list);
+bool split_stream_filename(TALLOC_CTX *ctx,
+			const char *filename_in,
+			char **filename_out,
+			char **streamname_out);
 
 /* The following definitions come from lib/dummyroot.c */
 
diff --git a/source3/lib/filename_util.c b/source3/lib/filename_util.c
index 9cd348a..1084c21 100644
--- a/source3/lib/filename_util.c
+++ b/source3/lib/filename_util.c
@@ -280,3 +280,45 @@ bool ea_list_has_invalid_name(struct ea_list *ea_list)
 	}
 	return false;
 }
+
+/****************************************************************************
+ Split an incoming name into tallocd filename and stream components.
+ Returns true on success, false on out of memory.
+****************************************************************************/
+
+bool split_stream_filename(TALLOC_CTX *ctx,
+				const char *filename_in,
+				char **filename_out,
+				char **streamname_out)
+{
+	const char *stream_name = NULL;
+	char *stream_out = NULL;
+	char *file_out = NULL;
+
+	stream_name = strchr_m(filename_in, ':');
+
+	if (stream_name) {
+		stream_out = talloc_strdup(ctx, stream_name);
+		if (stream_out == NULL) {
+			return false;
+		}
+		file_out = talloc_strndup(ctx,
+					filename_in,
+					PTR_DIFF(stream_name, filename_in));
+	} else {
+		file_out = talloc_strdup(ctx, filename_in);
+	}
+
+	if (file_out == NULL) {
+		TALLOC_FREE(stream_out);
+		return false;
+	}
+
+	if (filename_out) {
+		*filename_out = file_out;
+	}
+	if (streamname_out) {
+		*streamname_out = stream_out;
+	}
+	return true;
+}
-- 
2.5.0


>From 8d67152ce3234b77447ed21b1dc5980e951cb933 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 9 Mar 2016 15:45:55 -0800
Subject: [PATCH 2/6] s3:lib: Rewrite synthetic_smb_fname_split() to use
 split_stream_filename().

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/lib/filename_util.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/source3/lib/filename_util.c b/source3/lib/filename_util.c
index 1084c21..9aacc2e 100644
--- a/source3/lib/filename_util.c
+++ b/source3/lib/filename_util.c
@@ -70,35 +70,33 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx,
 }
 
 /**
- * XXX: This is temporary and there should be no callers of this once
- * smb_filename is plumbed through all path based operations.
+ * There are a few legitimate users of this.
  */
 struct smb_filename *synthetic_smb_fname_split(TALLOC_CTX *ctx,
 					       const char *fname,
 					       const SMB_STRUCT_STAT *psbuf)
 {
-	const char *stream_name = NULL;
+	char *stream_name = NULL;
 	char *base_name = NULL;
 	struct smb_filename *ret;
+	bool ok;
 
-	if (!lp_posix_pathnames()) {
-		stream_name = strchr_m(fname, ':');
-	}
-
-	/* Setup the base_name/stream_name. */
-	if (stream_name) {
-		base_name = talloc_strndup(ctx, fname,
-					   PTR_DIFF(stream_name, fname));
-	} else {
-		base_name = talloc_strdup(ctx, fname);
+	if (lp_posix_pathnames()) {
+		/* No stream name looked for. */
+		return synthetic_smb_fname(ctx, fname, NULL, NULL);
 	}
 
-	if (!base_name) {
+	ok = split_stream_filename(ctx,
+				fname,
+				&base_name,
+				&stream_name);
+	if (!ok) {
 		return NULL;
 	}
 
 	ret = synthetic_smb_fname(ctx, base_name, stream_name, psbuf);
 	TALLOC_FREE(base_name);
+	TALLOC_FREE(stream_name);
 	return ret;
 }
 
-- 
2.5.0


>From e79709264912bfed9cddcca5ca8185f5a2438dda Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 9 Mar 2016 15:50:02 -0800
Subject: [PATCH 3/6] s3:lib: Remove the const SMB_STRUCT_STAT * parameter from
 synthetic_smb_fname_split().

Only one caller uses this, and this can be handled externally.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/proto.h     |  3 +--
 source3/lib/filename_util.c |  5 ++---
 source3/smbd/filename.c     |  4 ++--
 source3/smbd/pysmbd.c       |  4 ++--
 source3/smbd/reply.c        |  2 +-
 source3/torture/cmd_vfs.c   | 14 +++++++-------
 6 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index b1ac7b0..9a03a95 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -1185,8 +1185,7 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx,
 					 const char *stream_name,
 					 const SMB_STRUCT_STAT *psbuf);
 struct smb_filename *synthetic_smb_fname_split(TALLOC_CTX *ctx,
-					       const char *fname,
-					       const SMB_STRUCT_STAT *psbuf);
+					       const char *fname);
 const char *smb_fname_str_dbg(const struct smb_filename *smb_fname);
 const char *fsp_str_dbg(const struct files_struct *fsp);
 const char *fsp_fnum_dbg(const struct files_struct *fsp);
diff --git a/source3/lib/filename_util.c b/source3/lib/filename_util.c
index 9aacc2e..025a943 100644
--- a/source3/lib/filename_util.c
+++ b/source3/lib/filename_util.c
@@ -73,8 +73,7 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx,
  * There are a few legitimate users of this.
  */
 struct smb_filename *synthetic_smb_fname_split(TALLOC_CTX *ctx,
-					       const char *fname,
-					       const SMB_STRUCT_STAT *psbuf)
+					       const char *fname)
 {
 	char *stream_name = NULL;
 	char *base_name = NULL;
@@ -94,7 +93,7 @@ struct smb_filename *synthetic_smb_fname_split(TALLOC_CTX *ctx,
 		return NULL;
 	}
 
-	ret = synthetic_smb_fname(ctx, base_name, stream_name, psbuf);
+	ret = synthetic_smb_fname(ctx, base_name, stream_name, NULL);
 	TALLOC_FREE(base_name);
 	TALLOC_FREE(stream_name);
 	return ret;
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 14eb53f..fc56b24 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -1423,11 +1423,11 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx,
 		ZERO_STRUCT(st);
 		st.st_ex_nlink = 1;
 		*pp_smb_fname = synthetic_smb_fname_split(ctx,
-							  name_in,
-							  &st);
+							  name_in);
 		if (*pp_smb_fname == NULL) {
 			return NT_STATUS_NO_MEMORY;
 		}
+		(*pp_smb_fname)->st = st;
 		return NT_STATUS_OK;
 	}
 
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 68bc3e7..41f0e3f 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -124,7 +124,7 @@ static NTSTATUS set_nt_acl_conn(const char *fname,
 	   so set our umask to 0 */
 	saved_umask = umask(0);
 
-	smb_fname = synthetic_smb_fname_split(fsp, fname, NULL);
+	smb_fname = synthetic_smb_fname_split(fsp, fname);
 	if (smb_fname == NULL) {
 		TALLOC_FREE(frame);
 		umask(saved_umask);
@@ -446,7 +446,7 @@ static PyObject *py_smbd_unlink(PyObject *self, PyObject *args, PyObject *kwargs
 		return NULL;
 	}
 
-	smb_fname = synthetic_smb_fname_split(frame, fname, NULL);
+	smb_fname = synthetic_smb_fname_split(frame, fname);
 	if (smb_fname == NULL) {
 		TALLOC_FREE(frame);
 		return PyErr_NoMemory();
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index bb91d77..673b735 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -6645,7 +6645,7 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 		 * component of the destination.
 		 */
 		smb_fname_orig_lcomp = synthetic_smb_fname_split(
-			ctx, smb_fname_dst->original_lcomp, NULL);
+			ctx, smb_fname_dst->original_lcomp);
 		if (smb_fname_orig_lcomp == NULL) {
 			status = NT_STATUS_NO_MEMORY;
 			TALLOC_FREE(fname_dst_lcomp_base_mod);
diff --git a/source3/torture/cmd_vfs.c b/source3/torture/cmd_vfs.c
index 4bd5417..a06947c 100644
--- a/source3/torture/cmd_vfs.c
+++ b/source3/torture/cmd_vfs.c
@@ -354,7 +354,7 @@ static NTSTATUS cmd_open(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc, c
 	}
 	fsp->conn = vfs->conn;
 
-	smb_fname = synthetic_smb_fname_split(NULL, argv[1], NULL);
+	smb_fname = synthetic_smb_fname_split(NULL, argv[1]);
 	if (smb_fname == NULL) {
 		TALLOC_FREE(fsp);
 		return NT_STATUS_NO_MEMORY;
@@ -584,12 +584,12 @@ static NTSTATUS cmd_rename(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc,
 		return NT_STATUS_OK;
 	}
 
-	smb_fname_src = synthetic_smb_fname_split(mem_ctx, argv[1], NULL);
+	smb_fname_src = synthetic_smb_fname_split(mem_ctx, argv[1]);
 	if (smb_fname_src == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	smb_fname_dst = synthetic_smb_fname_split(mem_ctx, argv[2], NULL);
+	smb_fname_dst = synthetic_smb_fname_split(mem_ctx, argv[2]);
 	if (smb_fname_dst == NULL) {
 		TALLOC_FREE(smb_fname_src);
 		return NT_STATUS_NO_MEMORY;
@@ -644,7 +644,7 @@ static NTSTATUS cmd_stat(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc, c
 		return NT_STATUS_OK;
 	}
 
-	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1], NULL);
+	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1]);
 	if (smb_fname == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -783,7 +783,7 @@ static NTSTATUS cmd_lstat(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc,
 		return NT_STATUS_OK;
 	}
 
-	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1], NULL);
+	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1]);
 	if (smb_fname == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -1043,7 +1043,7 @@ static NTSTATUS cmd_utime(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc,
 	ft.atime = convert_time_t_to_timespec(atoi(argv[2]));
 	ft.mtime = convert_time_t_to_timespec(atoi(argv[3]));
 
-	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1], NULL);
+	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1]);
 	if (smb_fname == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -1530,7 +1530,7 @@ static NTSTATUS cmd_set_nt_acl(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int a
 	}
 	fsp->conn = vfs->conn;
 
-	smb_fname = synthetic_smb_fname_split(NULL, argv[1], NULL);
+	smb_fname = synthetic_smb_fname_split(NULL, argv[1]);
 	if (smb_fname == NULL) {
 		TALLOC_FREE(fsp);
 		return NT_STATUS_NO_MEMORY;
-- 
2.5.0


>From 99eeade731e18bf75e59045273486326c05b7d19 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 9 Mar 2016 16:00:47 -0800
Subject: [PATCH 4/6] s3:lib: Move internal lp_posix_pathnames() call out of
 utility function synthetic_smb_fname_split().

Make it a passed in parameter instead.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/proto.h     |  3 ++-
 source3/lib/filename_util.c |  5 +++--
 source3/smbd/filename.c     |  3 ++-
 source3/smbd/pysmbd.c       |  8 ++++++--
 source3/smbd/reply.c        |  4 +++-
 source3/torture/cmd_vfs.c   | 28 +++++++++++++++++++++-------
 6 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 9a03a95..530a3af 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -1185,7 +1185,8 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx,
 					 const char *stream_name,
 					 const SMB_STRUCT_STAT *psbuf);
 struct smb_filename *synthetic_smb_fname_split(TALLOC_CTX *ctx,
-					       const char *fname);
+						const char *fname,
+						bool posix_path);
 const char *smb_fname_str_dbg(const struct smb_filename *smb_fname);
 const char *fsp_str_dbg(const struct files_struct *fsp);
 const char *fsp_fnum_dbg(const struct files_struct *fsp);
diff --git a/source3/lib/filename_util.c b/source3/lib/filename_util.c
index 025a943..6ee91ec 100644
--- a/source3/lib/filename_util.c
+++ b/source3/lib/filename_util.c
@@ -73,14 +73,15 @@ struct smb_filename *synthetic_smb_fname(TALLOC_CTX *mem_ctx,
  * There are a few legitimate users of this.
  */
 struct smb_filename *synthetic_smb_fname_split(TALLOC_CTX *ctx,
-					       const char *fname)
+						const char *fname,
+						bool posix_path)
 {
 	char *stream_name = NULL;
 	char *base_name = NULL;
 	struct smb_filename *ret;
 	bool ok;
 
-	if (lp_posix_pathnames()) {
+	if (posix_path) {
 		/* No stream name looked for. */
 		return synthetic_smb_fname(ctx, fname, NULL, NULL);
 	}
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index fc56b24..dffa71d 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -1423,7 +1423,8 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx,
 		ZERO_STRUCT(st);
 		st.st_ex_nlink = 1;
 		*pp_smb_fname = synthetic_smb_fname_split(ctx,
-							  name_in);
+					name_in,
+					(ucf_flags & UCF_POSIX_PATHNAMES));
 		if (*pp_smb_fname == NULL) {
 			return NT_STATUS_NO_MEMORY;
 		}
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 41f0e3f..4d95bcf 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -124,7 +124,9 @@ static NTSTATUS set_nt_acl_conn(const char *fname,
 	   so set our umask to 0 */
 	saved_umask = umask(0);
 
-	smb_fname = synthetic_smb_fname_split(fsp, fname);
+	smb_fname = synthetic_smb_fname_split(fsp,
+					fname,
+					lp_posix_pathnames());
 	if (smb_fname == NULL) {
 		TALLOC_FREE(frame);
 		umask(saved_umask);
@@ -446,7 +448,9 @@ static PyObject *py_smbd_unlink(PyObject *self, PyObject *args, PyObject *kwargs
 		return NULL;
 	}
 
-	smb_fname = synthetic_smb_fname_split(frame, fname);
+	smb_fname = synthetic_smb_fname_split(frame,
+					fname,
+					lp_posix_pathnames());
 	if (smb_fname == NULL) {
 		TALLOC_FREE(frame);
 		return PyErr_NoMemory();
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 673b735..3fb07fb 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -6645,7 +6645,9 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 		 * component of the destination.
 		 */
 		smb_fname_orig_lcomp = synthetic_smb_fname_split(
-			ctx, smb_fname_dst->original_lcomp);
+			ctx,
+			smb_fname_dst->original_lcomp,
+			lp_posix_pathnames());
 		if (smb_fname_orig_lcomp == NULL) {
 			status = NT_STATUS_NO_MEMORY;
 			TALLOC_FREE(fname_dst_lcomp_base_mod);
diff --git a/source3/torture/cmd_vfs.c b/source3/torture/cmd_vfs.c
index a06947c..7c49ce7 100644
--- a/source3/torture/cmd_vfs.c
+++ b/source3/torture/cmd_vfs.c
@@ -354,7 +354,9 @@ static NTSTATUS cmd_open(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc, c
 	}
 	fsp->conn = vfs->conn;
 
-	smb_fname = synthetic_smb_fname_split(NULL, argv[1]);
+	smb_fname = synthetic_smb_fname_split(NULL,
+					argv[1],
+					lp_posix_pathnames());
 	if (smb_fname == NULL) {
 		TALLOC_FREE(fsp);
 		return NT_STATUS_NO_MEMORY;
@@ -584,12 +586,16 @@ static NTSTATUS cmd_rename(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc,
 		return NT_STATUS_OK;
 	}
 
-	smb_fname_src = synthetic_smb_fname_split(mem_ctx, argv[1]);
+	smb_fname_src = synthetic_smb_fname_split(mem_ctx,
+					argv[1],
+					lp_posix_pathnames());
 	if (smb_fname_src == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	smb_fname_dst = synthetic_smb_fname_split(mem_ctx, argv[2]);
+	smb_fname_dst = synthetic_smb_fname_split(mem_ctx,
+					argv[2],
+					lp_posix_pathnames());
 	if (smb_fname_dst == NULL) {
 		TALLOC_FREE(smb_fname_src);
 		return NT_STATUS_NO_MEMORY;
@@ -644,7 +650,9 @@ static NTSTATUS cmd_stat(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc, c
 		return NT_STATUS_OK;
 	}
 
-	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1]);
+	smb_fname = synthetic_smb_fname_split(mem_ctx,
+					argv[1],
+					lp_posix_pathnames());
 	if (smb_fname == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -783,7 +791,9 @@ static NTSTATUS cmd_lstat(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc,
 		return NT_STATUS_OK;
 	}
 
-	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1]);
+	smb_fname = synthetic_smb_fname_split(mem_ctx,
+					argv[1],
+					lp_posix_pathnames());
 	if (smb_fname == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -1043,7 +1053,9 @@ static NTSTATUS cmd_utime(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int argc,
 	ft.atime = convert_time_t_to_timespec(atoi(argv[2]));
 	ft.mtime = convert_time_t_to_timespec(atoi(argv[3]));
 
-	smb_fname = synthetic_smb_fname_split(mem_ctx, argv[1]);
+	smb_fname = synthetic_smb_fname_split(mem_ctx,
+					argv[1],
+					lp_posix_pathnames());
 	if (smb_fname == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -1530,7 +1542,9 @@ static NTSTATUS cmd_set_nt_acl(struct vfs_state *vfs, TALLOC_CTX *mem_ctx, int a
 	}
 	fsp->conn = vfs->conn;
 
-	smb_fname = synthetic_smb_fname_split(NULL, argv[1]);
+	smb_fname = synthetic_smb_fname_split(NULL,
+					argv[1],
+					lp_posix_pathnames());
 	if (smb_fname == NULL) {
 		TALLOC_FREE(fsp);
 		return NT_STATUS_NO_MEMORY;
-- 
2.5.0


>From 76464246248a64fd55dcdfe27bec2a7a6c243274 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 9 Mar 2016 16:01:52 -0800
Subject: [PATCH 5/6] s3: smbd: Simplify logic inside rename_internals_fsp()
 part 1.

Use standard parent_dirname() function instead of hand-hacking
using strrchr_m(xxx, '/'). Next commit should enable removal
of synthetic_smb_fname_split().

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/reply.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 3fb07fb..81750fa 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -6622,20 +6622,19 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 	if (!conn->case_sensitive && conn->case_preserve &&
 	    strequal(fsp->fsp_name->base_name, smb_fname_dst->base_name) &&
 	    strequal(fsp->fsp_name->stream_name, smb_fname_dst->stream_name)) {
-		char *last_slash;
-		char *fname_dst_lcomp_base_mod = NULL;
+		char *fname_dst_parent = NULL;
+		const char *fname_dst_lcomp = NULL;
 		struct smb_filename *smb_fname_orig_lcomp = NULL;
 
 		/*
-		 * Get the last component of the destination name.
+		 * Split off the last component of the processed
+		 * destination name. We will compare this to
+		 * the split components of smb_fname_dst->original_lcomp.
 		 */
-		last_slash = strrchr_m(smb_fname_dst->base_name, '/');
-		if (last_slash) {
-			fname_dst_lcomp_base_mod = talloc_strdup(ctx, last_slash + 1);
-		} else {
-			fname_dst_lcomp_base_mod = talloc_strdup(ctx, smb_fname_dst->base_name);
-		}
-		if (!fname_dst_lcomp_base_mod) {
+		if (!parent_dirname(ctx,
+				smb_fname_dst->base_name,
+				&fname_dst_parent,
+				&fname_dst_lcomp)) {
 			status = NT_STATUS_NO_MEMORY;
 			goto out;
 		}
@@ -6650,32 +6649,30 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 			lp_posix_pathnames());
 		if (smb_fname_orig_lcomp == NULL) {
 			status = NT_STATUS_NO_MEMORY;
-			TALLOC_FREE(fname_dst_lcomp_base_mod);
+			TALLOC_FREE(fname_dst_parent);
 			goto out;
 		}
 
 		/* If the base names only differ by case, use original. */
-		if(!strcsequal(fname_dst_lcomp_base_mod,
+		if(!strcsequal(fname_dst_lcomp,
 			       smb_fname_orig_lcomp->base_name)) {
 			char *tmp;
 			/*
 			 * Replace the modified last component with the
 			 * original.
 			 */
-			if (last_slash) {
-				*last_slash = '\0'; /* Truncate at the '/' */
+			if (!ISDOT(fname_dst_parent)) {
 				tmp = talloc_asprintf(smb_fname_dst,
 					"%s/%s",
-					smb_fname_dst->base_name,
+					fname_dst_parent,
 					smb_fname_orig_lcomp->base_name);
 			} else {
-				tmp = talloc_asprintf(smb_fname_dst,
-					"%s",
+				tmp = talloc_strdup(smb_fname_dst,
 					smb_fname_orig_lcomp->base_name);
 			}
 			if (tmp == NULL) {
 				status = NT_STATUS_NO_MEMORY;
-				TALLOC_FREE(fname_dst_lcomp_base_mod);
+				TALLOC_FREE(fname_dst_parent);
 				TALLOC_FREE(smb_fname_orig_lcomp);
 				goto out;
 			}
@@ -6692,14 +6689,14 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 					    smb_fname_orig_lcomp->stream_name);
 			if (tmp == NULL) {
 				status = NT_STATUS_NO_MEMORY;
-				TALLOC_FREE(fname_dst_lcomp_base_mod);
+				TALLOC_FREE(fname_dst_parent);
 				TALLOC_FREE(smb_fname_orig_lcomp);
 				goto out;
 			}
 			TALLOC_FREE(smb_fname_dst->stream_name);
 			smb_fname_dst->stream_name = tmp;
 		}
-		TALLOC_FREE(fname_dst_lcomp_base_mod);
+		TALLOC_FREE(fname_dst_parent);
 		TALLOC_FREE(smb_fname_orig_lcomp);
 	}
 
-- 
2.5.0


>From 72714c5385d692ffd0e63365a22c5ded2a7f3f50 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 9 Mar 2016 16:12:00 -0800
Subject: [PATCH 6/6] s3: smbd: Simplify logic inside rename_internals_fsp()
 part 2

Removes the use of an extraneous 'struct smb_filename *'
which wasn't being created correctly, only as a place
holder for two char * pointers.

Use split_stream_filename() to create the char * pointers
directly and make it clearer what we're up to here.

The logic here is still complex, but I'm satified
it does the correct thing.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/reply.c | 60 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 21 deletions(-)

diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 81750fa..8a59b5d 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -6624,7 +6624,9 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 	    strequal(fsp->fsp_name->stream_name, smb_fname_dst->stream_name)) {
 		char *fname_dst_parent = NULL;
 		const char *fname_dst_lcomp = NULL;
-		struct smb_filename *smb_fname_orig_lcomp = NULL;
+		char *orig_lcomp_path = NULL;
+		char *orig_lcomp_stream = NULL;
+		bool ok = true;
 
 		/*
 		 * Split off the last component of the processed
@@ -6640,22 +6642,36 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 		}
 
 		/*
-		 * Create an smb_filename struct using the original last
-		 * component of the destination.
+		 * The original_lcomp component contains
+		 * the last_component of the path + stream
+		 * name (if a stream exists).
+		 *
+		 * Split off the stream name so we
+		 * can check them separately.
 		 */
-		smb_fname_orig_lcomp = synthetic_smb_fname_split(
-			ctx,
-			smb_fname_dst->original_lcomp,
-			lp_posix_pathnames());
-		if (smb_fname_orig_lcomp == NULL) {
-			status = NT_STATUS_NO_MEMORY;
+
+		if (fsp->posix_flags & FSP_POSIX_FLAGS_PATHNAMES) {
+			/* POSIX - no stream component. */
+			orig_lcomp_path = talloc_strdup(ctx,
+						smb_fname_dst->original_lcomp);
+			if (orig_lcomp_path == NULL) {
+				ok = false;
+			}
+		} else {
+			ok = split_stream_filename(ctx,
+					smb_fname_dst->original_lcomp,
+					&orig_lcomp_path,
+					&orig_lcomp_stream);
+		}
+
+		if (!ok) {
 			TALLOC_FREE(fname_dst_parent);
+			status = NT_STATUS_NO_MEMORY;
 			goto out;
 		}
 
 		/* If the base names only differ by case, use original. */
-		if(!strcsequal(fname_dst_lcomp,
-			       smb_fname_orig_lcomp->base_name)) {
+		if(!strcsequal(fname_dst_lcomp, orig_lcomp_path)) {
 			char *tmp;
 			/*
 			 * Replace the modified last component with the
@@ -6665,15 +6681,16 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 				tmp = talloc_asprintf(smb_fname_dst,
 					"%s/%s",
 					fname_dst_parent,
-					smb_fname_orig_lcomp->base_name);
+					orig_lcomp_path);
 			} else {
 				tmp = talloc_strdup(smb_fname_dst,
-					smb_fname_orig_lcomp->base_name);
+					orig_lcomp_path);
 			}
 			if (tmp == NULL) {
 				status = NT_STATUS_NO_MEMORY;
 				TALLOC_FREE(fname_dst_parent);
-				TALLOC_FREE(smb_fname_orig_lcomp);
+				TALLOC_FREE(orig_lcomp_path);
+				TALLOC_FREE(orig_lcomp_stream);
 				goto out;
 			}
 			TALLOC_FREE(smb_fname_dst->base_name);
@@ -6682,22 +6699,23 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
 
 		/* If the stream_names only differ by case, use original. */
 		if(!strcsequal(smb_fname_dst->stream_name,
-			       smb_fname_orig_lcomp->stream_name)) {
-			char *tmp = NULL;
+			       orig_lcomp_stream)) {
 			/* Use the original stream. */
-			tmp = talloc_strdup(smb_fname_dst,
-					    smb_fname_orig_lcomp->stream_name);
+			char *tmp = talloc_strdup(smb_fname_dst,
+					    orig_lcomp_stream);
 			if (tmp == NULL) {
 				status = NT_STATUS_NO_MEMORY;
 				TALLOC_FREE(fname_dst_parent);
-				TALLOC_FREE(smb_fname_orig_lcomp);
+				TALLOC_FREE(orig_lcomp_path);
+				TALLOC_FREE(orig_lcomp_stream);
 				goto out;
 			}
 			TALLOC_FREE(smb_fname_dst->stream_name);
-			smb_fname_dst->stream_name = tmp;
+			smb_fname_dst->stream_name = orig_lcomp_stream;
 		}
 		TALLOC_FREE(fname_dst_parent);
-		TALLOC_FREE(smb_fname_orig_lcomp);
+		TALLOC_FREE(orig_lcomp_path);
+		TALLOC_FREE(orig_lcomp_stream);
 	}
 
 	/*
-- 
2.5.0



More information about the samba-technical mailing list