Hello,
could you please add this to mainline? Getting EINVAL when an acl
becomes too large is quite confusing.
Index: linux-2.6.6-rc2/fs/ext2/acl.c
===================================================================
--- linux-2.6.6-rc2.orig/fs/ext2/acl.c 2004-04-20 23:29:46.000000000 +0200
+++ linux-2.6.6-rc2/fs/ext2/acl.c 2004-04-26 11:45:59.724792120 +0200
@@ -256,7 +256,7 @@
}
if (acl) {
if (acl->a_count > EXT2_ACL_MAX_ENTRIES)
- return -EINVAL;
+ return -ENOSPC;
value = ext2_acl_to_disk(acl, &size);
if (IS_ERR(value))
return (int)PTR_ERR(value);
Index: linux-2.6.6-rc2/fs/ext3/acl.c
===================================================================
--- linux-2.6.6-rc2.orig/fs/ext3/acl.c 2004-04-20 23:28:53.000000000 +0200
+++ linux-2.6.6-rc2/fs/ext3/acl.c 2004-04-26 11:46:05.143968280 +0200
@@ -260,7 +260,7 @@
}
if (acl) {
if (acl->a_count > EXT3_ACL_MAX_ENTRIES)
- return -EINVAL;
+ return -ENOSPC;
value = ext3_acl_to_disk(acl, &size);
if (IS_ERR(value))
return (int)PTR_ERR(value);
Thanks,
--
Andreas Gruenbacher <[email protected]>
SUSE Labs, SUSE LINUX AG
hi Andreas,
On Mon, Apr 26, 2004 at 12:27:58PM +0200, Andreas Gruenbacher wrote:
> Hello,
>
> could you please add this to mainline? Getting EINVAL when an acl
> becomes too large is quite confusing.
>
> if (acl) {
> if (acl->a_count > EXT2_ACL_MAX_ENTRIES)
> - return -EINVAL;
> + return -ENOSPC;
That seems an odd error code to change it to, since its not
related to the device running out of free space right? Maybe
use -E2BIG instead?
XFS has a similar check (different limit as you know), so is
also affected by this; could you update XFS at the same time
with whatever value gets chosen, if its not EINVAL? Or just
let me know what gets chosen & I'll fix it up later.
thanks.
--
Nathan
On Tue, 2004-04-27 at 03:24, Nathan Scott wrote:
> hi Andreas,
>
> On Mon, Apr 26, 2004 at 12:27:58PM +0200, Andreas Gruenbacher wrote:
> > Hello,
> >
> > could you please add this to mainline? Getting EINVAL when an acl
> > becomes too large is quite confusing.
> >
> > if (acl) {
> > if (acl->a_count > EXT2_ACL_MAX_ENTRIES)
> > - return -EINVAL;
> > + return -ENOSPC;
>
> That seems an odd error code to change it to, since its not
> related to the device running out of free space right? Maybe
> use -E2BIG instead?
I don't see a problem with giving ENOSPC this particular meaning here.
The error number used must be among those defined for NFSv3, so E2BIG
won't do.
> XFS has a similar check (different limit as you know), so is
> also affected by this; could you update XFS at the same time
> with whatever value gets chosen, if its not EINVAL? Or just
> let me know what gets chosen & I'll fix it up later.
I think this patch is correct for xfs. Nathan, would you mind to
double-check? Thanks.
Index: linux-2.6.6-rc2/fs/xfs/xfs_acl.c
===================================================================
--- linux-2.6.6-rc2.orig/fs/xfs/xfs_acl.c
+++ linux-2.6.6-rc2/fs/xfs/xfs_acl.c
@@ -148,10 +148,7 @@ posix_acl_xattr_to_xfs(
return EINVAL;
}
}
- if (xfs_acl_invalid(dest))
- return EINVAL;
-
- return 0;
+ return xfs_acl_invalid(dest);
}
/*
@@ -249,10 +246,9 @@ xfs_acl_vget(
if (!size) {
error = -posix_acl_xattr_size(XFS_ACL_MAX_ENTRIES);
} else {
- if (xfs_acl_invalid(xfs_acl)) {
- error = EINVAL;
+ error = xfs_acl_invalid(xfs_acl);
+ if (error)
goto out;
- }
if (kind == _ACL_TYPE_ACCESS) {
vattr_t va;
@@ -590,7 +586,7 @@ xfs_acl_invalid(
goto acl_invalid;
if (aclp->acl_cnt > XFS_ACL_MAX_ENTRIES)
- goto acl_invalid;
+ return ENOSPC;
for (i = 0; i < aclp->acl_cnt; i++) {
entry = &aclp->acl_entry[i];
Thanks,
--
Andreas Gruenbacher <[email protected]>
SUSE Labs, SUSE LINUX AG
On Mon, Apr 26, 2004 at 12:27:58PM +0200, Andreas Gruenbacher wrote:
> could you please add this to mainline? Getting EINVAL when an acl
> becomes too large is quite confusing.
On my system, at least, "man acl_set_file" does explicitly say that
EINVAL is returned in this case. Whether that should be considered a
bug in the documentation or the code I don't know....
--Bruce Fields
On Tue, 2004-04-27 at 20:32, J. Bruce Fields wrote:
> On Mon, Apr 26, 2004 at 12:27:58PM +0200, Andreas Gruenbacher wrote:
> > could you please add this to mainline? Getting EINVAL when an acl
> > becomes too large is quite confusing.
>
> On my system, at least, "man acl_set_file" does explicitly say that
> EINVAL is returned in this case. Whether that should be considered a
> bug in the documentation or the code I don't know....
Indeed ... looking at POSIX 1003.1e draft 17 I now see that this
function is indeed supposed to return EINVAL in this case. I was
assuming that this case was undefined, but that was wrong. So let's
leave the code as is -- sorry.
Cheers,
--
Andreas Gruenbacher <[email protected]>
SUSE Labs, SUSE LINUX AG