Re: [PATCH 2/2] Add SMB 2 support for getting and setting SACLs

[Boris Protopopov]

No objections on my part as far as adding 'if (info & SACL_SECINFO) return'.

I had originally those flags sent from the caller, but that was
confusing at the top level (e.g. in cifsacl.c), so I have opted to
passing only "extra" flags ("additional" was already taken).

-------------
...
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -3315,9 +3315,9 @@ get_smb2_acl(struct cifs_sb_info *cifs_sb,
> >         struct cifs_ntsd *pntsd = NULL;
> >         struct cifsFileInfo *open_file = NULL;
> >
> > -       if (inode)
> > +       if (inode && !(info & SACL_SECINFO))
> >                 open_file = find_readable_file(CIFS_I(inode), true);
> > -       if (!open_file)
> > +       if (!open_file || (info & SACL_SECINFO))
> >                 return get_smb2_acl_by_path(cifs_sb, path, pacllen, info);
> >
> >         pntsd = get_smb2_acl_by_fid(cifs_sb, &open_file->fid, pacllen, info);
> Why not have an if (info & SACL_SECINFO) return
> get_smb2_acl_by_path... right at the beginning?
> Looks cleaner that way IMO.
>
...
> > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> > index 0aeb63694306..b207e1eb6803 100644
> > --- a/fs/cifs/smb2pdu.c
> > +++ b/fs/cifs/smb2pdu.c
> > @@ -3472,8 +3472,10 @@ SMB311_posix_query_info(const unsigned int xid, struct cifs_tcon *tcon,
> >  int
> >  SMB2_query_acl(const unsigned int xid, struct cifs_tcon *tcon,
> >                u64 persistent_fid, u64 volatile_fid,
> > -              void **data, u32 *plen, u32 additional_info)
> > +              void **data, u32 *plen, u32 extra_info)
> >  {
> > +       __u32 additional_info = OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO |
> > +                               extra_info;
> I still prefer having these flags set by the caller. That way, the
> caller has the flexibility to query only the subset needed.
> --
> -Shyam