Patch: S3-Add VFS Functions for setting and getting DOS Attributes

[Uri Simchoni]

On 03/21/2016 12:53 AM, Richard Sharpe wrote:
> Hi folks,
>
> Here is the patch for consideration.
>
> I ran a make test on this and that seems to have succeeded.
>
Sorry for not noticing those things earlier - I could spot them in the 
preview patch, but I do have some comments:

*handle,+ bool (*set_dos_attributes_fn)(struct vfs_handle_struct *handle,
+ const struct smb_filename *smb_fname,
+ uint32_t dosmode);
+

Since those functions don't have POSIX semantics, wouldn't it be better 
to have them return NTSTATUS?



+static bool vfswrap_fset_dos_attributes(struct vfs_handle_struct + 
                                   struct files_struct *fsp,
+                                       uint32_t dosmode)
+{
+       return set_ea_dos_attribute(handle->conn, fsp->fsp_name, dosmode);
+}

I think the default here should be to come up with something that 
eventually calls fsetxattr, assuming the file is open. Otherwise there's 
no point in having this extra function.


@@ -893,8 +895,7 @@ NTSTATUS file_set_sparse(connection_struct *conn,
         }

         /* Store the DOS attributes in an EA. */
-       if (!set_ea_dos_attribute(conn, fsp->fsp_name,
-                                 new_dosmode)) {
+       if (!SMB_VFS_FSET_DOS_ATTRIBUTES(conn, fsp, new_dosmode)) {
                 if (errno == 0) {
                         errno = EIO;
                 }

(shows the benefit of returning NTSTATUS but also - )

Assuming the SMB_VFS_FSET_DOS_ATTRIBUTES becomes fd-based, do we have 
enough guarantees here that this isn't a "stat open", i.e. that the fsp 
points to an open file? (should really get my mind wrapped around those 
stat opens...:))

Thanks,
Uri.