Re: [Updated PATCH v2] drm: modify drm_global_item_ref to avoid two times of writing ref->object

[Huang Rui <ray.huang@amd.com>]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel