lkml.org 
[lkml]   [2010]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Patch] mqueue: fix the bad code in sys_mq_open()
    On Thu, Feb 25, 2010 at 09:40:23PM +0800, Am??rico Wang wrote:
    >
    > Fix bad code in ipc/mqueue.c.
    >
    > Inspired by Andr?? Goddard Rosa's patch.
    >
    > a) do_create() and do_open() should not release the resources which
    > are not acquired within themselves, it's their caller's work;

    Sorry, NAK. Resources are either consumed by opened file (i.e. struct
    file now is the holder of mnt/dentry) or released; in either case,
    caller has given them up. Your variant is broken in its current form
    and will be more complicated than existing code if you fix it.

    > b) Fix an fd leak;

    Fixed by original patch.

    > c) The goto label 'out_upsem' doesn't make any sense, rename to
    > 'out_unlock';
    >
    > d) mntget() should be called before looking up dentry within
    > ->mnt->mnt_root;

    Not really, since ->mnt doesn't change (and remains pinned) for the
    entire lifetime of ipc_ns.

    > e) When dealing with failure case, we should release the resources
    > in a reversed order of acquiring, so reorder the goto labels;
    >
    > f) Remove some unused labels due to reorder.

    I wouldn't say it's cleaner that way.

    Anyway, I've applied the patchset as posted; if you want to do something
    else, do that on top of it. I really object against your (a), the rest
    is more or less a matter of taste. (a) is just plain wrong - we want
    uniform behaviour from the caller's POV and it means that mnt/dentry should
    always be given up from caller's POV. Either moved into new struct file,
    or dropped.


    \
     
     \ /
      Last update: 2010-03-03 20:57    [W:0.044 / U:0.904 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site