All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	xemul@openvz.org, xemul@parallels.com, avagin@openvz.org,
	kosaki.motohiro@gmail.com, mingo@elte.hu, hpa@zytor.com,
	tglx@linutronix.de, glommer@parallels.com, andi@firstfloor.org,
	tj@kernel.org, matthltc@us.ibm.com, penberg@kernel.org,
	eric.dumazet@gmail.com, segoon@openwall.com, adobriyan@gmail.com,
	Valdis.Kletnieks@vt.edu
Subject: Re: [RFC c/r 2/4] [RFC] syscalls, x86: Add __NR_kcmp syscall v7
Date: Fri, 27 Jan 2012 13:19:20 -0800	[thread overview]
Message-ID: <m1bopozvvr.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <20120127205026.GT11086@moon> (Cyrill Gorcunov's message of "Sat, 28 Jan 2012 00:50:26 +0400")

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

  reply	other threads:[~2012-01-27 21:16 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-27 17:53 [RFC c/r 0/4] [RFC c/r 0/@total@] A pile in c/r sake Cyrill Gorcunov
2012-01-27 17:53 ` [RFC c/r 1/4] fs, proc: Introduce /proc/<pid>/task/<tid>/children entry v9 Cyrill Gorcunov
2012-01-27 17:53 ` [RFC c/r 2/4] [RFC] syscalls, x86: Add __NR_kcmp syscall v7 Cyrill Gorcunov
2012-01-27 18:05   ` H. Peter Anvin
2012-01-27 18:11     ` Cyrill Gorcunov
2012-01-27 18:15   ` Andi Kleen
2012-01-27 18:24     ` Cyrill Gorcunov
2012-01-27 18:30       ` H. Peter Anvin
2012-01-28 17:19         ` Michael Kerrisk
2012-01-28 17:34           ` Cyrill Gorcunov
2012-01-28 17:36             ` Cyrill Gorcunov
2012-01-27 18:31       ` Andi Kleen
2012-01-27 18:40         ` Cyrill Gorcunov
2012-01-27 19:40           ` Andi Kleen
2012-01-27 20:55             ` Eric W. Biederman
2012-01-27 18:40   ` Eric W. Biederman
2012-01-27 18:45     ` Cyrill Gorcunov
2012-01-27 19:10     ` Cyrill Gorcunov
2012-01-27 19:37   ` Vasiliy Kulikov
2012-01-27 19:59     ` hpanvin@gmail.com
2012-01-27 20:07       ` Cyrill Gorcunov
2012-01-27 20:19   ` KOSAKI Motohiro
2012-01-27 20:33     ` Eric W. Biederman
2012-01-27 20:50       ` Cyrill Gorcunov
2012-01-27 21:19         ` Eric W. Biederman [this message]
2012-01-27 20:34     ` Glauber Costa
2012-01-27 20:37       ` H. Peter Anvin
2012-01-27 20:47     ` Cyrill Gorcunov
2012-01-27 17:53 ` [RFC c/r 3/4] c/r: procfs: add arg_start/end, env_start/end and exit_code members to /proc/$pid/stat Cyrill Gorcunov
2012-01-27 18:29   ` Kees Cook
2012-01-27 20:00   ` KOSAKI Motohiro
2012-01-27 20:10     ` Cyrill Gorcunov
2012-01-27 17:53 ` [RFC c/r 4/4] c/r: prctl: Extend PR_SET_MM to set up more mm_struct entries Cyrill Gorcunov
2012-01-27 18:37   ` Kees Cook
2012-01-27 18:43     ` Cyrill Gorcunov
2012-01-27 20:31       ` KOSAKI Motohiro
2012-01-27 20:28     ` KOSAKI Motohiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1bopozvvr.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=avagin@openvz.org \
    --cc=eric.dumazet@gmail.com \
    --cc=glommer@parallels.com \
    --cc=gorcunov@openvz.org \
    --cc=hpa@zytor.com \
    --cc=kosaki.motohiro@gmail.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthltc@us.ibm.com \
    --cc=mingo@elte.hu \
    --cc=penberg@kernel.org \
    --cc=segoon@openwall.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=xemul@openvz.org \
    --cc=xemul@parallels.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.