[PATCH] Begin VFS cleanup - start to remove SMB_VFS_READ calls.

Jeremy Allison jra at samba.org
Tue May 1 23:29:42 UTC 2018


Hi Ralph and other interested parties,

Here is the first (simple) stage of the patchset
to remove SMB_VFS_READ. It removes the fiction
that Samba will work in the absense of pread
(that hasn't been true for a while), removes
an incorrect recursion call back into the VFS
from the default VFS pread code and replaces
it with it's POSIX equivalent (this is to allow
the default VFS pread call to work against a filesystem
FIFO, which returns ESPIPE on a pread()) and
then moves a function to the only caller and
makes it static.

The ugly cleanups start after this easier to
review patchset :-).

Please review and push if happy !

Cheers,

	Jeremy.
-------------- next part --------------
From 262315fbedda949675c177260970052f58b487e0 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 30 Apr 2018 09:50:04 -0700
Subject: [PATCH 1/3] s3: VFS: default: Remove fallback if we don't have
 HAVE_PREAD set. Samba doesn't work without pread.

Start of the changes to remove synchronous VFS read.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_default.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 6de0329b337..6e8a99690cf 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -641,28 +641,8 @@ static ssize_t vfswrap_pread(vfs_handle_struct *handle, files_struct *fsp, void
 	}
 
 #else /* HAVE_PREAD */
-	off_t   curr;
-	int lerrno;
-
-	curr = SMB_VFS_LSEEK(fsp, 0, SEEK_CUR);
-	if (curr == -1 && errno == ESPIPE) {
-		/* Maintain the fiction that pipes can be seeked (sought?) on. */
-		result = SMB_VFS_READ(fsp, data, n);
-		fsp->fh->pos = 0;
-		return result;
-	}
-
-	if (SMB_VFS_LSEEK(fsp, offset, SEEK_SET) == -1) {
-		return -1;
-	}
-
-	errno = 0;
-	result = SMB_VFS_READ(fsp, data, n);
-	lerrno = errno;
-
-	SMB_VFS_LSEEK(fsp, curr, SEEK_SET);
-	errno = lerrno;
-
+	errno = ENOSYS;
+	result = -1;
 #endif /* HAVE_PREAD */
 
 	return result;
-- 
2.14.1


From 2c8a77c97e8e52df3330cd3425bdc152ea870ec1 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 30 Apr 2018 09:51:34 -0700
Subject: [PATCH 2/3] s3: VFS: default: Remove recursion into the VFS inside
 the default pread call.

We already know we're at the POSIX level here.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_default.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 6e8a99690cf..56dd5c77218 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -636,7 +636,7 @@ static ssize_t vfswrap_pread(vfs_handle_struct *handle, files_struct *fsp, void
 
 	if (result == -1 && errno == ESPIPE) {
 		/* Maintain the fiction that pipes can be seeked (sought?) on. */
-		result = SMB_VFS_READ(fsp, data, n);
+		result = sys_read(fsp->fh->fd, data, n);
 		fsp->fh->pos = 0;
 	}
 
-- 
2.14.1


From 87fed1c4dc0b8da0055ee78fbcdfcd00f7516dcb Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 30 Apr 2018 10:15:49 -0700
Subject: [PATCH 3/3] s3: VFS: Default. Move vfs_read_data() out of
 source3/smbd/vfs.c to the printing code, which is the only caller.

Make static.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/printing/nt_printing.c | 28 ++++++++++++++++++++++++++++
 source3/smbd/proto.h           |  1 -
 source3/smbd/vfs.c             | 25 -------------------------
 3 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c
index 54357b787f3..bf54fd4e6b3 100644
--- a/source3/printing/nt_printing.c
+++ b/source3/printing/nt_printing.c
@@ -311,6 +311,34 @@ const char *get_short_archi(const char *long_archi)
 	return archi_table[i].short_archi;
 }
 
+/****************************************************************************
+ Read data from fsp on the vfs.
+ (note: EINTR re-read differs from vfs_write_data)
+****************************************************************************/
+
+static ssize_t vfs_read_data(files_struct *fsp, char *buf, size_t byte_count)
+{
+	size_t total=0;
+
+	while (total < byte_count) {
+		ssize_t ret = SMB_VFS_READ(fsp, buf + total,
+					byte_count - total);
+
+		if (ret == 0) {
+			return total;
+		}
+		if (ret == -1) {
+			if (errno == EINTR) {
+				continue;
+			} else {
+				return -1;
+			}
+		}
+		total += ret;
+	}
+	return (ssize_t)total;
+}
+
 /****************************************************************************
  Version information in Microsoft files is held in a VS_VERSION_INFO structure.
  There are two case to be covered here: PE (Portable Executable) and NE (New
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 4417595df3a..81350469c28 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1228,7 +1228,6 @@ void sys_utmp_yield(const char *username, const char *hostname,
 bool vfs_init_custom(connection_struct *conn, const char *vfs_object);
 bool smbd_vfs_init(connection_struct *conn);
 NTSTATUS vfs_file_exist(connection_struct *conn, struct smb_filename *smb_fname);
-ssize_t vfs_read_data(files_struct *fsp, char *buf, size_t byte_count);
 ssize_t vfs_write_data(struct smb_request *req,
 			files_struct *fsp,
 			const char *buffer,
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index f659c8f2783..47abf45496b 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -398,31 +398,6 @@ NTSTATUS vfs_file_exist(connection_struct *conn, struct smb_filename *smb_fname)
 	return NT_STATUS_OBJECT_NAME_NOT_FOUND;
 }
 
-/****************************************************************************
- Read data from fsp on the vfs. (note: EINTR re-read differs from vfs_write_data)
-****************************************************************************/
-
-ssize_t vfs_read_data(files_struct *fsp, char *buf, size_t byte_count)
-{
-	size_t total=0;
-
-	while (total < byte_count)
-	{
-		ssize_t ret = SMB_VFS_READ(fsp, buf + total,
-					   byte_count - total);
-
-		if (ret == 0) return total;
-		if (ret == -1) {
-			if (errno == EINTR)
-				continue;
-			else
-				return -1;
-		}
-		total += ret;
-	}
-	return (ssize_t)total;
-}
-
 /****************************************************************************
  Write data to a fd on the vfs.
 ****************************************************************************/
-- 
2.14.1



More information about the samba-technical mailing list