On Tue, 9 Nov 2010 12:52:16 -0600
Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
> On Tue, Nov 9, 2010 at 11:40 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > On Tue, 9 Nov 2010 08:35:02 -0600
> > shirishpargaonkar@xxxxxxxxx wrote:
> >
> >> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >>
> >>
> >> Modify get/set_cifs_acl* calls to reutrn error code and percolate the
> >> error code up to the caller.
> >>
> >>
> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> >> ---
> >> fs/cifs/cifsacl.c | 46 ++++++++++++++++++++++++++++++----------------
> >> 1 files changed, 30 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> >> index c9b4792..003588a 100644
> >> --- a/fs/cifs/cifsacl.c
> >> +++ b/fs/cifs/cifsacl.c
> >> @@ -568,8 +568,11 @@ static struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *cifs_sb,
> >>
> >> cifs_put_tlink(tlink);
> >>
> >> - cFYI(1, "GetCIFSACL rc = %d ACL len %d", rc, *pacllen);
> >> - return pntsd;
> >> + cFYI(1, "%s: rc = %d ACL len %d", __func__, rc, *pacllen);
> >> + if (rc)
> >> + return ERR_PTR(rc);
> >> + else
> > ^^^^
> > This else isn't needed
> >
> >> + return pntsd;
> >> }
> >>
> >> static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
> >> @@ -591,19 +594,19 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
> >> rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, READ_CONTROL, 0,
> >> &fid, &oplock, NULL, cifs_sb->local_nls,
> >> cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
> >> - if (rc) {
> >> - cERROR(1, "Unable to open file to get ACL");
> >> - goto out;
> >> + if (!rc) {
> >> + rc = CIFSSMBGetCIFSACL(xid, tcon, fid, &pntsd, pacllen);
> >> + CIFSSMBClose(xid, tcon, fid);
> >> }
> >>
> >> - rc = CIFSSMBGetCIFSACL(xid, tcon, fid, &pntsd, pacllen);
> >> - cFYI(1, "GetCIFSACL rc = %d ACL len %d", rc, *pacllen);
> >> -
> >> - CIFSSMBClose(xid, tcon, fid);
> >> - out:
> >> cifs_put_tlink(tlink);
> >> FreeXid(xid);
> >> - return pntsd;
> >> +
> >> + cFYI(1, "%s: rc = %d ACL len %d", __func__, rc, *pacllen);
> >> + if (rc)
> >> + return ERR_PTR(rc);
> >> + else
> > ^^^^
> > Also not needed...
> >
> >> + return pntsd;
> >> }
> >>
> >> /* Retrieve an ACL from the server */
> >> @@ -711,12 +714,18 @@ cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
> >> pntsd = get_cifs_acl(cifs_sb, inode, path, &acllen);
> >>
> >> /* if we can retrieve the ACL, now parse Access Control Entries, ACEs */
> >> - if (pntsd)
> >> + if (!pntsd)
> >> + cERROR(1, "%s: NULL sec desc", __func__);
> > ^^^^^^^^
> > Will this ever be a null pointer? I don't think it will.
> > It's either going to be an ERR_PTR or a real cifs_ntsd.
> > I think that check isn't needed.
>
> get_cifs_acl_by_pid/path can return NULL when the return thus
>
> if (IS_ERR(tlink))
> return NULL;
>
That needs to be fixed too. Don't return NULL there, return
ERR_CAST(tlink) instead.
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
