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