Messages in this thread | | | From | (Eric W. Biederman) | Subject | Re: [RFC c/r 2/4] [RFC] syscalls, x86: Add __NR_kcmp syscall v7 | Date | Fri, 27 Jan 2012 13:19:20 -0800 |
| |
Cyrill Gorcunov <gorcunov@openvz.org> writes:
> On Fri, Jan 27, 2012 at 12:33:07PM -0800, Eric W. Biederman wrote: >> KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> writes: >> >> + ret = kcmp_ptr((long)filp1, (long)filp2, KCMP_FILE); >> >> + else >> >> + ret = -ENOENT; >> > >> > If my remember is correct, Andrew pointed out EINVAL is better than ENOENT. >> >> Ah yes. And really what it should be is >> if (!filp1 || !filp2) >> return -EBADF; >> >> At least EBADF is what you return if it is your process that doesn't >> have the filedescriptor. >> > > Eric, I've sent out version with > > if (filp1 && filp2) > ... > else > ret = -EBADF; > > maybe you're lookin into previous version?
Yeah. Comments and patch passing in the night. It looks like you have it right in your latest patch.
>> >> + KCMP_SYSVSEM); >> >> +#else >> >> + ret = -EINVAL; >> > >> > ENOTSUP is better, I think. because of, EINVAL implicitly mean _caller_ is wrong. >> > but in this case, it is not bad. only the kernel doesn't have enough >> > feature. >> >> Careful a type compiled out should in principle match a type whose >> support has not been implemented. That is the default case should match >> what happens when you don't compile in sysvipc support. > > I don't get it :) Will -EINVAL be enough or not?
At the present time the only way we can get -EINVAL by not supporting a type so there is no pressing need for something different.
However in the general case EINVAL is a pretty generic failure mode and having something more precise that you can use to figure out what you did wrong when calling a system call tends to help a great deal.
So I am favor of using a better error code if EOPNOTSUP or ENOTTY if we can convince ourselves it is the proper error code.
Mostly it is bike shedding but it is a detail that getting it right will help users of this interface in the long run.
Eric
| |