mirror of https://lore.kernel.org/lkml/
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome