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 12:33:07 -0800 |
| |
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> writes:
> On 1/27/2012 12:53 PM, Cyrill Gorcunov wrote: >> + switch (type) { >> + case KCMP_FILE: { >> + struct file *filp1, *filp2; >> + >> + filp1 = get_file_raw_ptr(task1, idx1); >> + filp2 = get_file_raw_ptr(task2, idx2); >> + >> + if (filp1 && filp2) >> + 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.
>> + break; >> + case KCMP_SYSVSEM: >> +#ifdef CONFIG_SYSVIPC >> + ret = kcmp_ptr((long)task1->sysvsem.undo_list, >> + (long)task2->sysvsem.undo_list, >> + 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.
> >> + goto err; > > you don't need err label at all. > > >> +#endif >> + break; >> + default: >> + ret = -EINVAL; >> + goto err; >> + } >> + >> +err: >> + put_task_struct(task1); >> + put_task_struct(task2); >> + >> + return ret; >> +}
Eric
| |