[EXTERNAL] Re: Review request: changes in samba vxfs plugin

[Ralph Böhme]

Hi Pooja,

On Sat, Aug 05, 2017 at 04:26:00PM +0000, Pooja Mahadik wrote:
> Hi Jeremy,
> 
> Thanks a lot for the review.
> 
> Can someone please do the second review for this patch?

sorry, but nack.

I don't think this would actually build...

Can you please check the attached suggested fixes?

Cheerio!
-slow
-------------- next part --------------
From 339e2fca08c37db70ecc71d12b02ac9679c266a3 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 9 Aug 2017 15:01:58 +0200
Subject: [PATCH 1/5] lib_vxfs: include header file

---
 source3/modules/lib_vxfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/modules/lib_vxfs.c b/source3/modules/lib_vxfs.c
index accd739..1cd3522 100644
--- a/source3/modules/lib_vxfs.c
+++ b/source3/modules/lib_vxfs.c
@@ -22,6 +22,7 @@
 #include "smbd/smbd.h"
 #include "system/filesys.h"
 #include "string.h"
+#include "vfs_vxfs.h"
 
 /*
  * Available under GPL at
-- 
2.9.4


From 2c5300f0ae942bf1dc2aac9023c8c234612c5355 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 9 Aug 2017 15:02:43 +0200
Subject: [PATCH 2/5] FIXUP: remove surplus parenthesis

Did this actually build?
---
 source3/modules/lib_vxfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/modules/lib_vxfs.c b/source3/modules/lib_vxfs.c
index 1cd3522..81ff35e 100644
--- a/source3/modules/lib_vxfs.c
+++ b/source3/modules/lib_vxfs.c
@@ -236,7 +236,7 @@ int vxfs_setwxattr_path(const char *path)
 	}
 
 	ret = vxfs_setwxattr_fd(fd);
-	DBG_DEBUG("ret = %d\n", ret));
+	DBG_DEBUG("ret = %d\n", ret);
 	close(fd);
 
 	return ret;
-- 
2.9.4


From 88a28209f3d76230d8ab61fa8e44434db4f011d0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 9 Aug 2017 15:03:58 +0200
Subject: [PATCH 3/5] FIXUP: lib_vxfs.c: more explicit error checking, cf
 README.coding

---
 source3/modules/lib_vxfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/modules/lib_vxfs.c b/source3/modules/lib_vxfs.c
index 81ff35e..f9394d6 100644
--- a/source3/modules/lib_vxfs.c
+++ b/source3/modules/lib_vxfs.c
@@ -216,7 +216,7 @@ int vxfs_setwxattr_fd(int fd)
 	}
 	ret = vxfs_setwxattr_fd_func(fd);
 	DBG_DEBUG("ret = %d\n", ret);
-	if (ret) {
+	if (ret != 0) {
 		errno = ret;
 		ret = -1;
 	}
@@ -251,7 +251,7 @@ int vxfs_clearwxattr_fd(int fd)
 	}
 	ret = vxfs_clearwxattr_fd_func(fd);
 	DBG_DEBUG("ret = %d\n", ret);
-	if (ret) {
+	if (ret != 0) {
 		errno = ret;
 		ret = -1;
 	}
@@ -286,7 +286,7 @@ int vxfs_checkwxattr_fd(int fd)
 	}
 	ret = vxfs_checkwxattr_fd_func(fd);
 	DBG_DEBUG("ret = %d\n", ret);
-	if (ret) {
+	if (ret != 0) {
 		errno = ret;
 		ret = -1;
 	}
-- 
2.9.4


From 9b7caeea684180f51c55d80e4aa42fc1d7e8a802 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 9 Aug 2017 15:05:34 +0200
Subject: [PATCH 4/5] FIXUP: vfs_vxfs: use a bool if it's a bool

---
 source3/modules/vfs_vxfs.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_vxfs.c b/source3/modules/vfs_vxfs.c
index 803c1bf..24f734d 100644
--- a/source3/modules/vfs_vxfs.c
+++ b/source3/modules/vfs_vxfs.c
@@ -833,7 +833,8 @@ static NTSTATUS vxfs_set_ea_dos_attributes(struct vfs_handle_struct *handle,
 					   uint32_t dosmode)
 {
 	NTSTATUS	err;
-	int		ret = 0, attrset = 0;
+	int		ret = 0;
+	bool            attrset = false;
 
 	DBG_DEBUG("Entered function\n");
 
@@ -854,7 +855,7 @@ static NTSTATUS vxfs_set_ea_dos_attributes(struct vfs_handle_struct *handle,
 				return map_nt_error_from_unix(errno);
 			}
 		} else {
-			attrset = 1;
+			attrset = true;
 		}
 	}
 	err = SMB_VFS_NEXT_SET_DOS_ATTRIBUTES(handle, smb_fname, dosmode);
@@ -879,6 +880,7 @@ static NTSTATUS vxfs_fset_ea_dos_attributes(struct vfs_handle_struct *handle,
 {
 	NTSTATUS	err;
 	int		ret = 0;
+	bool            attrset = false;
 
 	DBG_DEBUG("Entered function\n");
 
@@ -899,7 +901,7 @@ static NTSTATUS vxfs_fset_ea_dos_attributes(struct vfs_handle_struct *handle,
 				return map_nt_error_from_unix(errno);
 			}
 		} else {
-			attrset = 1;
+			attrset = true;
 		}
 	}
 	err = SMB_VFS_NEXT_FSET_DOS_ATTRIBUTES(handle, fsp, dosmode);
-- 
2.9.4


From eba6ac51e9ba822af1680cb69f59b0b89f0e71f7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 9 Aug 2017 15:06:10 +0200
Subject: [PATCH 5/5] MAYBE/FIXUP: vfs_vxfs: reduce indentetion

---
 source3/modules/vfs_vxfs.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/source3/modules/vfs_vxfs.c b/source3/modules/vfs_vxfs.c
index 24f734d..ed48f5e 100644
--- a/source3/modules/vfs_vxfs.c
+++ b/source3/modules/vfs_vxfs.c
@@ -863,10 +863,8 @@ static NTSTATUS vxfs_set_ea_dos_attributes(struct vfs_handle_struct *handle,
 		if (attrset) {
 			ret = vxfs_clearwxattr_path(smb_fname->base_name);
 			DBG_DEBUG("ret:%d\n", ret);
-			if (ret == -1) {
-				if ((errno != ENOENT)) {
-					return map_nt_error_from_unix(errno);
-				}
+			if ((ret == -1) && (errno != ENOENT)) {
+				return map_nt_error_from_unix(errno);
 			}
 		}
 	}
@@ -909,10 +907,8 @@ static NTSTATUS vxfs_fset_ea_dos_attributes(struct vfs_handle_struct *handle,
 		if (attrset) {
 			ret = vxfs_clearwxattr_fd(fsp->fh->fd);
 			DBG_DEBUG("ret:%d\n", ret);
-			if (ret == -1) {
-				if (errno != ENOENT) {
-					return map_nt_error_from_unix(errno);
-				}
+			if ((ret == -1) && (errno != ENOENT)) {
+				return map_nt_error_from_unix(errno);
 			}
 		}
 	}
-- 
2.9.4