All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: chrisw@sous-sol.org, oleg@redhat.com, roland@redhat.com,
	linux-kernel@vger.kernel.org, viro@ZenIV.linux.org.uk
Subject: Re: [patch 1/2] ptrace, security: rename ptrace_may_access => ptrace_access_check
Date: Mon, 11 May 2009 15:39:49 +0200	[thread overview]
Message-ID: <20090511133949.GA28533@elte.hu> (raw)
In-Reply-To: <20090507125526.c257c23d.akpm@linux-foundation.org>


* Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 7 May 2009 11:49:47 +0200
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > The ptrace_may_access() methods are named confusingly - some 
> > variants return a bool, while the security subsystem methods have a 
> > retval convention.
> > 
> > Rename it to ptrace_access_check, to reduce the confusion factor. A 
> > followup patch eliminates the bool usage.
> 
> s/may_access/access_check/ is a poor change.  The new name conveys 
> less information than the old one.
> 
> It's quite clear what the return value from "may_access" means.  

it isnt clear at all. In fact there's two variants: one that returns 
'int' and one that returns 'bool' - the two have inverted values.

> It's less clear what the return value from a function called 
> "access_check" means.
>
> Switching to something like ptrace_task_accessible() or 
> ptrace_may_access_task() would be better.
> 
> This happens quite often.  The string "check" in the name of a 
> predicate function is a red flag.

I disagree. To repeat the argument i made in this thread, the 'may' 
suggests/attracts a logical value, i.e. yes or no, or boolean. But 
that goes against the desire of actual call sites wanting a Linux 
retval.

I.e. any function name that can be plain-English answered with: 
'yes' or 'no' is a red flag for a retval function.

You cannot answer ptrace_access_check() with 'yes' or 'no'. You 
could if it was ptrace_access_ok() or ptrace_may_access.

	Ingo

  reply	other threads:[~2009-05-11 13:41 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
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 [this message]
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=20090511133949.GA28533@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 \
    --cc=viro@ZenIV.linux.org.uk \
    /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.