2008-10-20 06:17:25

by Krishna Kumar2

Subject: [PATCH 4/4] nfsd: Minor cleanup of _get_posix_acl

From: Krishna Kumar <[email protected]>

Rewrite error case to not check twice for !buflen. Compile tested.

Signed-off-by: Krishna Kumar <[email protected]>
---
fs/nfsd/vfs.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff -ruNp linux-2.6.27.org/fs/nfsd/vfs.c linux-2.6.27.new/fs/nfsd/vfs.c
--- linux-2.6.27.org/fs/nfsd/vfs.c 2008-10-20 10:47:19.000000000 +0530
+++ linux-2.6.27.new/fs/nfsd/vfs.c 2008-10-20 10:48:32.000000000 +0530
@@ -503,10 +503,11 @@ _get_posix_acl(struct dentry *dentry, ch
int buflen;

buflen = nfsd_getxattr(dentry, key, &buf);
- if (!buflen)
- buflen = -ENODATA;
- if (buflen <= 0)
+ if (buflen <= 0) {
+ if (!buflen)
+ buflen = -ENODATA;
return ERR_PTR(buflen);
+ }

pacl = posix_acl_from_xattr(buf, buflen);
kfree(buf);

2008-10-20 21:03:01

by J. Bruce Fields

Subject: Re: [PATCH 4/4] nfsd: Minor cleanup of _get_posix_acl

On Mon, Oct 20, 2008 at 11:47:21AM +0530, Krishna Kumar wrote:
> From: Krishna Kumar <[email protected]>
>
> Rewrite error case to not check twice for !buflen. Compile tested.

Is the current code really a problem?

Possibly more interesting to look into:

- Can nfsd_getxattr actually return NULL?
- Why do we need to return ENODATA in that case?
(nfsd_get_posix_acl, for example, seems to just end up
returning -EINVAL in that case.)

--b.

>
> Signed-off-by: Krishna Kumar <[email protected]>
> ---
> fs/nfsd/vfs.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff -ruNp linux-2.6.27.org/fs/nfsd/vfs.c linux-2.6.27.new/fs/nfsd/vfs.c
> --- linux-2.6.27.org/fs/nfsd/vfs.c 2008-10-20 10:47:19.000000000 +0530
> +++ linux-2.6.27.new/fs/nfsd/vfs.c 2008-10-20 10:48:32.000000000 +0530
> @@ -503,10 +503,11 @@ _get_posix_acl(struct dentry *dentry, ch
> int buflen;
>
> buflen = nfsd_getxattr(dentry, key, &buf);
> - if (!buflen)
> - buflen = -ENODATA;
> - if (buflen <= 0)
> + if (buflen <= 0) {
> + if (!buflen)
> + buflen = -ENODATA;
> return ERR_PTR(buflen);
> + }
>
> pacl = posix_acl_from_xattr(buf, buflen);
> kfree(buf);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html