Messages in this thread |  | | | Date | Wed, 3 Mar 2010 19:54:49 +0000 | | From | Al Viro <> | | Subject | Re: [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.
|  |