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

[Richard Sharpe]

On Sun, Mar 20, 2016 at 10:06 PM, Uri Simchoni <uri at samba.org> wrote:
> 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?

Yeah, since I suggested such a thing some time ago :-)

Having looked at it again, I think what I can do is to have the
default in vfs_default convert any UNIX errors to NTSTATUS and return
that.

In some cases in the current callers I will then need to just check
for an error, in others minor changes are needed.

Then any implementer can return their own errors.

> +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.

Sure ... there are no callers currently and it was easier to simply
convert it to the other call in the default.

That will need more thought. Perhaps implement another version of the
underlying 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.
>



-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)