On Mon, 8 Nov 2010 09:47:18 -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 | 84 ++++++++++++++++++++++++++++++----------------------
> 1 files changed, 48 insertions(+), 36 deletions(-)
>
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index c9b4792..8c260b9 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -552,38 +552,40 @@ static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd,
> return rc;
> }
>
> -static struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *cifs_sb,
> - __u16 fid, u32 *pacllen)
> +static int
> +get_cifs_acl_by_fid(struct cifs_sb_info *cifs_sb,
> + __u16 fid, u32 *pacllen, struct cifs_ntsd **pntsd)
^^^^^^^^^^^
Dealing with pointers to pointers like this is cumbersome and
inefficient. Why not just have it return a struct pointer like before
or a ERR_PTR converted error?
> {
> - struct cifs_ntsd *pntsd = NULL;
> - int xid, rc;
> + int xid;
> + int rc = 0;
> struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
>
> if (IS_ERR(tlink))
> - return NULL;
> + PTR_ERR(tlink);
^^^^^^^^^^^^^^^
bug?
>
> xid = GetXid();
> - rc = CIFSSMBGetCIFSACL(xid, tlink_tcon(tlink), fid, &pntsd, pacllen);
> + rc = CIFSSMBGetCIFSACL(xid, tlink_tcon(tlink), fid, pntsd, pacllen);
> FreeXid(xid);
>
> cifs_put_tlink(tlink);
>
> cFYI(1, "GetCIFSACL rc = %d ACL len %d", rc, *pacllen);
> - return pntsd;
> + return rc;
> }
>
> -static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
> - const char *path, u32 *pacllen)
> +static int
> +get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb, const char *path,
> + u32 *pacllen, struct cifs_ntsd **pntsd)
> {
> - struct cifs_ntsd *pntsd = NULL;
> int oplock = 0;
> - int xid, rc;
> + int xid;
> + int rc = 0;
> __u16 fid;
> struct cifsTconInfo *tcon;
> struct tcon_link *tlink = cifs_sb_tlink(cifs_sb);
>
> if (IS_ERR(tlink))
> - return NULL;
> + return PTR_ERR(tlink);
>
> tcon = tlink_tcon(tlink);
> xid = GetXid();
> @@ -596,32 +598,34 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
> goto out;
> }
>
> - rc = CIFSSMBGetCIFSACL(xid, tcon, fid, &pntsd, pacllen);
> + 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;
> + return rc;
> }
>
> /* Retrieve an ACL from the server */
> -static struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
> - struct inode *inode, const char *path,
> - u32 *pacllen)
> +static int
> +get_cifs_acl(struct cifs_sb_info *cifs_sb, struct inode *inode,
> + const char *path, u32 *pacllen, struct cifs_ntsd **pntsd)
> {
> - struct cifs_ntsd *pntsd = NULL;
> + int rc = 0;
> struct cifsFileInfo *open_file = NULL;
>
> if (inode)
> open_file = find_readable_file(CIFS_I(inode), true);
^^^^^^^^^^
This predates your patch, but under what conditions would we
call this function without an inode? If you're cleaning up the
prototype for this function, then I think there are some
unneeded arguments here.
--
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
