lkml.org 
[lkml]   [2012]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 2/2] cred: change keyctl_session_to_parent() to use task_work_queue()
Date
Oleg Nesterov <oleg@redhat.com> wrote:

> Change keyctl_session_to_parent() to use task_work_queue() and
> move key_replace_session_keyring() logic into task_work->func().

I'm generally okay with this, but there are a couple of issues with the patch.

> +static void replace_session_keyring(struct task_work *twork)

Can you keep this in process_keys.c please? Then everything that actually
updates a process's keyrings is done there. Admittedly, on that basis, you
can argue that I should move a chunk of keyctl_session_to_parent() there too.

And, also, can you please keep the "key_" on the front of the name?

> long keyctl_session_to_parent(void)
> {
> -#ifdef TIF_NOTIFY_RESUME

Unless TIF_NOTIFY_RESUME is defined, this operation cannot be performed and
should generate an error. I don't see how this happens now.

> + if (!task_work_queue(parent, newwork))

I hate this type of construct. "if not function()" indicating the function
succeeded. Can you make it "== 0" instead? Also, shouldn't we tell the user
that it failed?

David


\
 
 \ /
  Last update: 2012-04-12 11:33    [from the cache]
©2003-2011 Jasper Spaans