[lkml]   [2010]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
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.048 / U:32.068 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site