[lkml]   [2012]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH v2 2/2] cred: change keyctl_session_to_parent() to use task_work_queue()
    Oleg Nesterov <> 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?


     \ /
      Last update: 2012-04-12 11:33    [W:0.019 / U:81.848 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site