All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Roland McGrath <roland@redhat.com>
Cc: Oleg Nesterov <oleg@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Chris Wright <chrisw@sous-sol.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 3/3a] ptrace: add _ptrace_may_access()
Date: Thu, 7 May 2009 10:17:48 +0200	[thread overview]
Message-ID: <20090507081748.GB12285@elte.hu> (raw)
In-Reply-To: <20090506203224.684C7FC39E@magilla.sf.frob.com>


* Roland McGrath <roland@redhat.com> wrote:

> > Something like the patch below allows the reuse of the locked 
> > version of __ptrace_may_access and pushes the int->bool 
> > conversion into an inline.
> 
> I think it would be cleaner and safe/simple enough to invert the 
> public ptrace_may_access() to just return the int and invert the ! 
> on all the callers (all one in fs/proc/task_mmu.c and all four in 
> fs/proc/base.c).

hm, i considered that briefly and rejected the idea because 
something that says 'may' in its name is generally assumed to be a 
logic function-ish thing.

I.e. in such case: ptrace_may_access() or task_is_traced() people 
sub-consciously assume that 0 means "no", non-0 means "yes". So in 
that sense i liked the bool wrapper and preserved that in the 
inline.

We do have the retval==0-means-success convention in a number of 
APIs, but APIs that include a verb in their names assert some sort 
of property all have bool behavior.

(The underscore itself signals some special property - so there the 
deviation from the usual conventions isnt a big problem.)

This might sound like a nuance, but it really matters in the grand 
scheme of things. So IMHO inverting the logic is a step backwards - 
beyond the needless churn as well that it causes in various 
subsystems.

	Ingo

  parent reply	other threads:[~2009-05-07  8:19 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-05 22:47 [PATCH 3/3] ptrace: do not use task_lock() for attach Oleg Nesterov
2009-05-06  2:08 ` Roland McGrath
2009-05-06  8:00 ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Ingo Molnar
2009-05-06 20:32   ` Roland McGrath
2009-05-06 20:47     ` Christoph Hellwig
2009-05-06 21:09       ` Roland McGrath
2009-05-07  8:19       ` Ingo Molnar
2009-05-07  8:17     ` Ingo Molnar [this message]
2009-05-06 23:53   ` Oleg Nesterov
2009-05-07  0:21     ` Roland McGrath
2009-05-07  6:36       ` Oleg Nesterov
2009-05-07  8:20         ` Ingo Molnar
2009-05-07  8:31           ` Oleg Nesterov
2009-05-07  8:38             ` Ingo Molnar
2009-05-07  8:49               ` [patch] security: rename ptrace_may_access => ptrace_access_check Ingo Molnar
2009-05-07  9:19                 ` Oleg Nesterov
2009-05-07  9:27                   ` Ingo Molnar
2009-05-07  8:57               ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Chris Wright
2009-05-07  9:04                 ` Ingo Molnar
2009-05-07  9:20                   ` Chris Wright
2009-05-07  9:54                     ` James Morris
2009-05-07 10:20                       ` your mail Ingo Molnar
2009-05-07 11:37                         ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-07 14:17                           ` Ingo Molnar
2009-06-23 14:14                           ` Oleg Nesterov
2009-06-23 17:49                             ` Christoph Hellwig
2009-06-23 19:24                               ` [PATCH 0/1] mm_for_maps: simplify, use ptrace_may_access() Oleg Nesterov
2009-06-23 19:25                                 ` [PATCH 1/1] " Oleg Nesterov
2009-06-24  3:06                                   ` Serge E. Hallyn
2009-06-24 14:21                                     ` James Morris
2009-06-24  9:25                                   ` Roland McGrath
2009-06-24 14:37                                     ` Oleg Nesterov
2009-06-24  1:08                             ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-08  3:27                         ` your mail Casey Schaufler
2009-06-24 14:19                       ` security: rename ptrace_may_access => ptrace_access_check James Morris
2009-05-07  9:31                   ` [RFC PATCH 3/3a] ptrace: add _ptrace_may_access() Ingo Molnar
2009-05-07  9:49                     ` [patch 1/2] ptrace, security: rename ptrace_may_access => ptrace_access_check Ingo Molnar
2009-05-07 18:47                       ` Roland McGrath
2009-05-07 19:55                       ` Andrew Morton
2009-05-11 13:39                         ` Ingo Molnar
2009-05-11 18:51                           ` Andrew Morton
2009-05-15  1:10                           ` Américo Wang
2009-05-15 19:34                             ` Ingo Molnar
2009-05-07  9:50                     ` [patch 2/2] ptrace: turn ptrace_access_check() into a retval function Ingo Molnar
2009-05-07 18:47                       ` Roland McGrath
2009-05-06 22:46 ` [PATCH 3/3] ptrace: do not use task_lock() for attach Chris Wright
2009-05-06 23:13   ` Oleg Nesterov
2009-05-06 23:27     ` Chris Wright
2009-05-06 23:48       ` James Morris
2009-05-07  1:17         ` Roland McGrath
2009-05-08 12:18           ` David Howells

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=20090507081748.GB12285@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=chrisw@sous-sol.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=roland@redhat.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.