this is ok with me
On Tue, Oct 06, 2015 at 12:25:57PM +0200, Benny Lofgren wrote: > Hi Nicholas, > > On 2015-10-06 09:56, Nicholas Marriott wrote: > > I am not convinced, changing errno like this is gratuitous. We actually > > do do it elsewhere, but IMO that is unnecessary too. > > That's fair enough. One can argue whether a certain error code is better > suited than another until the cows come home, but it isn't that big of a > deal. > > I only feel strongly about it because it always helps developers to be > as consistent and clear as possible in both the API itself and its > documentation. But I realize that this is an edge case anyway, one that > you're not likely to encounter unless you've done something seriously > wrong, so the ambiguity the current behaviour introduces is hardly a > problem in practice. > > For the record I did a quick grep through the source for instances of > copyinstr() and there were 28 in the kernel, of which all but five where > path-related. Of the other five, one is in kern_tame.c, one in > kern_ktrace.c, two in if.c and one in kern_prot.c (that one does the > same error code transformation I suggested). > > > In any case, I hope you at least agree with me that the documentation > should reflect actual behaviour? :-) I've updated my diff to tame.2 to > describe the error returns more accurately, minus the errno change. (And > I hope I got it right, I suck at mdoc...) > > > Regards, > > /Benny > > PS. Thanks to Nayden Markatchev and Matthew Martin who both off-list > pointed me to Teds blog post where he reasons about the argument being a > string. > > > > Index: tame.2 > =================================================================== > RCS file: /cvs/src/lib/libc/sys/tame.2,v > retrieving revision 1.31 > diff -u -p -r1.31 tame.2 > --- tame.2 4 Oct 2015 20:47:16 -0000 1.31 > +++ tame.2 6 Oct 2015 10:13:13 -0000 > @@ -424,17 +424,24 @@ will fail if: > .Bl -tag -width Er > .It Bq Er EFAULT > .Fa paths > +or one of its elements, or > +.Fa request > points outside the process's allocated address space. > .It Bq Er ENAMETOOLONG > An element of > .Fa paths > -is too large, or prepending > +is too large, prepending > .Fa cwd > to it would exceed > .Dv PATH_MAX > -bytes. > +bytes, or > +.Fa request > +is too long. > .It Bq Er EPERM > -This process is attempting to increase permissions. > +This process is attempting to increase its permissions. > +.It Bq Er EINVAL > +.Ar request > +is malformed or contains invalid keywords. > .It Bq Er E2BIG > The > .Ar paths
