On Wed, Sep 07, 2016 at 04:02:47PM +0800, Koenig, Christian wrote:
Am 07.09.2016 um 09:54 schrieb Huang Rui:
On Wed, Sep 07, 2016 at 09:12:29AM +0200, Christian K?nig wrote:
Am 07.09.2016 um 07:24 schrieb Huang Rui:
snip.
++item->refcount;
- ref->object = item->object;
- mutex_unlock(&item->mutex);
- return 0;
- goto out;
A goto in the not error path is a bit unusual. Since out_err is only used once couldn't you just move the code into the if and then goto to out as well?
Alternative you could just duplicate the mutex release in the non-error path.
Actually, I also have a little concern with "goto" in non-error path. But as Sean's suggestion, use "goto" can avoid a duplicate mutex release, you know.
I would certainly prefer the duplicate mutex release.
@Sean, if you don't have concern with adding a mutex release below, I will update it in V3.
++item->refcount; mutex_unlock(&item->mutex); return 0;out_err: kfree(ref->object); ref->object = NULL; out:
BTW: I would name those labels error_free and error_unlock to make clear that they are for error handling and what they are supposed to do.
OK, will rename the label in V3.
Thanks, Rui