On Sat, Apr 16, 2005 at 11:22:18AM +0200, Jan Kopriva wrote:
> Sorry for the mess in previous email. Here it is again and clean :)

It looked interesting, you must be using some cross-referencing browser.

> >It is mapped in srvproc.cc. Lines bellow shows the place:
> >
> >02871         if (AL_Internalize ((AL_ExternalAccessList) AccessList 
> >->SeqBody, newACL) != 0) {
> >02872             SLog (0, "CheckSetACLSemantics: ACL internalize 
> >failed (%x.%x.%x)",
> >02873                     Fid .Volume, Fid .Vnode, Fid .Unique);
> >02874             return(EINVAL);
> >02875         }
> >
> >
> >In AL_Internalize, there is a following line:
> >
> >if (p + m > AL_MaxExtEntries) return(-1);
> >
> >That is why the execution in case of too big ACL, never gets to the 
> >code right below the previous branch:

Ah, I looked everywhere _after_ we returned E2BIG, but not before.
Actually I think ENOSPC might a better error when we're can't add the
ACL to the vnode.

> >I agree. I actually overlooked the second check in srvproc.cc :)
> >However it is executed only in case of correct ACL length thus noone
> >sees E2BIG error. Also, my intention was not let the cfs to perform
> >the long data path if it is able to detect incorrect ACL length. But
> >cfs is not time critical and let the server take care about the
> >return values is definitely better design.

And it allows us to update the server implementation without having to
remember to change all the other intermediate checks.

> >I can add a function right before  AL_Internalize() that just checks 
> >for the length and returns the E2BIG.  The second check can be wiped out.

I changed AL_Internalize to return ENOSPC if we can't fit the ACL,
ENOENT if the username isn't found in the pdb database or EINVAL in
other cases, I think those only happen when the acls are badly
formatted. And ofcourse propagate the error back to the client, who
ofcourse passes it back to cfs. I'll commit the change to CVS once I've
separated the diff, accidentally changed a tree with other stuff that
isn't ready.

> >Later
> >
> >Jan

Feels like I'm talking to myself ;)

Jan