From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
Subject: Re: [PATCH 03/10] cifs: reorganize get_cifs_acl
Date: Thu, 28 May 2009 10:23:03 -0500
Message-ID: <4a4634330905280823v2f32966dre737b06293d3fbb4@mail.gmail.com>
References: <1243456447-11176-1-git-send-email-jlayton@redhat.com>
	<1243456447-11176-4-git-send-email-jlayton@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Cc: linux-fsdevel@vger.kernel.org, linux-cifs-client@lists.samba.org
To: Jeff Layton <jlayton@redhat.com>
Return-path: <linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org>
In-Reply-To: <1243456447-11176-4-git-send-email-jlayton@redhat.com>
List-Unsubscribe: <https://lists.samba.org/mailman/listinfo/linux-cifs-client>,
	<mailto:linux-cifs-client-request@lists.samba.org?subject=unsubscribe>
List-Archive: <http://lists.samba.org/archive/linux-cifs-client>
List-Post: <mailto:linux-cifs-client@lists.samba.org>
List-Help: <mailto:linux-cifs-client-request@lists.samba.org?subject=help>
List-Subscribe: <https://lists.samba.org/mailman/listinfo/linux-cifs-client>,
	<mailto:linux-cifs-client-request@lists.samba.org?subject=subscribe>
Sender: linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org
Errors-To: linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org
List-Id: linux-fsdevel.vger.kernel.org

On Wed, May 27, 2009 at 3:34 PM, Jeff Layton <jlayton@redhat.com> wrote:
> From: Christoph Hellwig <hch@infradead.org>
>
> Thus spake Christoph:
>
> "But this whole set_cifs_acl function is a real mess anyway and needs
> some splitting up."
>
> With this change too, it's possible to call acl_to_uid_mode() with a
> NULL inode pointer. That (or something close to it) will eventually be
> necessary when cifs_get_inode_info is reorganized.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> =A0fs/cifs/cifsacl.c =A0 | =A0100 ++++++++++++++++++++++++++-------------=
-----------
> =A0fs/cifs/cifsproto.h | =A0 =A04 +-
> =A0fs/cifs/inode.c =A0 =A0 | =A0 =A02 +-
> =A03 files changed, 55 insertions(+), 51 deletions(-)
>
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index 57ecdc8..7f8e6c4 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -552,67 +552,66 @@ static int build_sec_desc(struct cifs_ntsd *pntsd, =
struct cifs_ntsd *pnntsd,
> =A0 =A0 =A0 =A0return rc;
> =A0}
>
> -
> -/* Retrieve an ACL from the server */
> -static struct cifs_ntsd *get_cifs_acl(u32 *pacllen, struct inode *inode,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0const char *path, const __u16 *pfid)
> +static struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *cifs_s=
b,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 __u16 fid, u32 *pacllen)
> =A0{
> - =A0 =A0 =A0 struct cifsFileInfo *open_file =3D NULL;
> - =A0 =A0 =A0 bool unlock_file =3D false;
> - =A0 =A0 =A0 int xid;
> - =A0 =A0 =A0 int rc =3D -EIO;
> - =A0 =A0 =A0 __u16 fid;
> - =A0 =A0 =A0 struct super_block *sb;
> - =A0 =A0 =A0 struct cifs_sb_info *cifs_sb;
> =A0 =A0 =A0 =A0struct cifs_ntsd *pntsd =3D NULL;
> + =A0 =A0 =A0 int xid, rc;
>
> - =A0 =A0 =A0 cFYI(1, ("get mode from ACL for %s", path));
> + =A0 =A0 =A0 xid =3D GetXid();
> + =A0 =A0 =A0 rc =3D CIFSSMBGetCIFSACL(xid, cifs_sb->tcon, fid, &pntsd, p=
acllen);
> + =A0 =A0 =A0 FreeXid(xid);
>
> - =A0 =A0 =A0 if (inode =3D=3D NULL)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
>
> - =A0 =A0 =A0 xid =3D GetXid();
> - =A0 =A0 =A0 if (pfid =3D=3D NULL)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 open_file =3D find_readable_file(CIFS_I(ino=
de));
> - =A0 =A0 =A0 else
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 fid =3D *pfid;
> + =A0 =A0 =A0 cFYI(1, ("GetCIFSACL rc =3D %d ACL len %d", rc, *pacllen));
> + =A0 =A0 =A0 return pntsd;
> +}
>
> - =A0 =A0 =A0 sb =3D inode->i_sb;
> - =A0 =A0 =A0 if (sb =3D=3D NULL) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 FreeXid(xid);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> - =A0 =A0 =A0 }
> - =A0 =A0 =A0 cifs_sb =3D CIFS_SB(sb);
> +static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_=
sb,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 const char *path, u32 *pacllen)
> +{
> + =A0 =A0 =A0 struct cifs_ntsd *pntsd =3D NULL;
> + =A0 =A0 =A0 int oplock =3D 0;
> + =A0 =A0 =A0 int xid, rc;
> + =A0 =A0 =A0 __u16 fid;
>
> - =A0 =A0 =A0 if (open_file) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 unlock_file =3D true;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 fid =3D open_file->netfid;
> - =A0 =A0 =A0 } else if (pfid =3D=3D NULL) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 int oplock =3D 0;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* open file */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 rc =3D CIFSSMBOpen(xid, cifs_sb->tcon, path=
, FILE_OPEN,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 READ_CONTRO=
L, 0, &fid, &oplock, NULL,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cifs_sb->lo=
cal_nls, cifs_sb->mnt_cifs_flags &
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 CIFS_MOUNT_MAP_SPECIAL_CHR);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (rc !=3D 0) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cERROR(1, ("Unable to open =
file to get ACL"));
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 FreeXid(xid);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 xid =3D GetXid();
> +
> + =A0 =A0 =A0 rc =3D CIFSSMBOpen(xid, cifs_sb->tcon, path, FILE_OPEN, REA=
D_CONTROL, 0,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&fid, &oplock, NULL, cif=
s_sb->local_nls,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0cifs_sb->mnt_cifs_flags =
& CIFS_MOUNT_MAP_SPECIAL_CHR);
> + =A0 =A0 =A0 if (rc) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 cERROR(1, ("Unable to open file to get ACL"=
));
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out;
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0rc =3D CIFSSMBGetCIFSACL(xid, cifs_sb->tcon, fid, &pntsd, =
pacllen);
> =A0 =A0 =A0 =A0cFYI(1, ("GetCIFSACL rc =3D %d ACL len %d", rc, *pacllen))=
;
> - =A0 =A0 =A0 if (unlock_file =3D=3D true) /* find_readable_file incremen=
ts ref count */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 atomic_dec(&open_file->wrtPending);
> - =A0 =A0 =A0 else if (pfid =3D=3D NULL) /* if opened above we have to cl=
ose the handle */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 CIFSSMBClose(xid, cifs_sb->tcon, fid);
> - =A0 =A0 =A0 /* else handle was passed in by caller */
>
> + =A0 =A0 =A0 CIFSSMBClose(xid, cifs_sb->tcon, fid);
> + out:
> =A0 =A0 =A0 =A0FreeXid(xid);
> =A0 =A0 =A0 =A0return pntsd;
> =A0}
>
> +/* Retrieve an ACL from the server */
> +static struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 struct inode *inode, const char *path,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 u32 *pacllen)
> +{
> + =A0 =A0 =A0 struct cifs_ntsd *pntsd =3D NULL;
> + =A0 =A0 =A0 struct cifsFileInfo *open_file =3D NULL;
> +
> + =A0 =A0 =A0 if (inode)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 open_file =3D find_readable_file(CIFS_I(ino=
de));
> + =A0 =A0 =A0 if (!open_file)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return get_cifs_acl_by_path(cifs_sb, path, =
pacllen);
> +
> + =A0 =A0 =A0 pntsd =3D get_cifs_acl_by_fid(cifs_sb, open_file->netfid, p=
acllen);
> + =A0 =A0 =A0 atomic_dec(&open_file->wrtPending);
> + =A0 =A0 =A0 return pntsd;
> +}
> +
> =A0/* Set an ACL on the server */
> =A0static int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct ino=
de *inode, const char *path)
> @@ -668,14 +667,19 @@ static int set_cifs_acl(struct cifs_ntsd *pnntsd, _=
_u32 acllen,
> =A0}
>
> =A0/* Translate the CIFS ACL (simlar to NTFS ACL) for a file into mode bi=
ts */
> -void acl_to_uid_mode(struct inode *inode, const char *path, const __u16 =
*pfid)
> +void acl_to_uid_mode(struct cifs_sb_info *cifs_sb, struct inode *inode,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char *path, const __u16 *p=
fid)
> =A0{
> =A0 =A0 =A0 =A0struct cifs_ntsd *pntsd =3D NULL;
> =A0 =A0 =A0 =A0u32 acllen =3D 0;
> =A0 =A0 =A0 =A0int rc =3D 0;
>
> =A0 =A0 =A0 =A0cFYI(DBG2, ("converting ACL to mode for %s", path));
> - =A0 =A0 =A0 pntsd =3D get_cifs_acl(&acllen, inode, path, pfid);
> +
> + =A0 =A0 =A0 if (pfid)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pntsd =3D get_cifs_acl_by_fid(cifs_sb, *pfi=
d, &acllen);
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 pntsd =3D get_cifs_acl(cifs_sb, inode, path=
, &acllen);
>
> =A0 =A0 =A0 =A0/* if we can retrieve the ACL, now parse Access Control En=
tries, ACEs */
> =A0 =A0 =A0 =A0if (pntsd)
> @@ -698,7 +702,7 @@ int mode_to_acl(struct inode *inode, const char *path=
, __u64 nmode)
> =A0 =A0 =A0 =A0cFYI(DBG2, ("set ACL from mode for %s", path));
>
> =A0 =A0 =A0 =A0/* Get the security descriptor */
> - =A0 =A0 =A0 pntsd =3D get_cifs_acl(&secdesclen, inode, path, NULL);
> + =A0 =A0 =A0 pntsd =3D get_cifs_acl(CIFS_SB(inode->i_sb), inode, path, &=
secdesclen);
>
> =A0 =A0 =A0 =A0/* Add three ACEs for owner, group, everyone getting rid o=
f
> =A0 =A0 =A0 =A0 =A0 other ACEs as chmod disables ACEs and set the securit=
y descriptor */
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index d542cf1..f945232 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -108,8 +108,8 @@ extern int cifs_get_inode_info(struct inode **pinode,
> =A0extern int cifs_get_inode_info_unix(struct inode **pinode,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const unsigned char *searc=
h_path,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct super_block *sb, in=
t xid);
> -extern void acl_to_uid_mode(struct inode *inode, const char *path,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const __u16 *pfid);
> +extern void acl_to_uid_mode(struct cifs_sb_info *cifs_sb, struct inode *=
inode,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const char *path, c=
onst __u16 *pfid);
> =A0extern int mode_to_acl(struct inode *inode, const char *path, __u64);
>
> =A0extern int cifs_mount(struct super_block *, struct cifs_sb_info *, cha=
r *,
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 42d6e0f..7fb9694 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -626,7 +626,7 @@ int cifs_get_inode_info(struct inode **pinode,
> =A0 =A0 =A0 =A0/* fill in 0777 bits from ACL */
> =A0 =A0 =A0 =A0if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0cFYI(1, ("Getting mode bits from ACL"));
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 acl_to_uid_mode(inode, full_path, pfid);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 acl_to_uid_mode(cifs_sb, inode, full_path, =
pfid);
> =A0 =A0 =A0 =A0}
> =A0#endif
> =A0 =A0 =A0 =A0if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> --
> 1.6.2.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" =
in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>

Looks good.