From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH] iio/accel/bmc150: Improve unlocking of a mutex in two functions To: Hans de Goede , linux-iio@vger.kernel.org Cc: Hartmut Knaack , Jonathan Cameron , Lars-Peter Clausen , Srinivas Pandruvada , LKML , kernel-janitors@vger.kernel.org References: <66d582a4-a77e-cd78-4215-49587ec2259e@users.sourceforge.net> From: SF Markus Elfring Message-ID: Date: Wed, 25 Oct 2017 18:15:32 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 List-ID: > IMHO, if you do this, you should rework the function so that there is a single unlock call > at the end, not a separate one in in error label. Thanks for your update suggestion. Does it indicate that I may propose similar source code adjustments in this software area? > Could e.g. change this: > >         ret = bmc150_accel_set_power_state(data, false); >         mutex_unlock(&data->mutex); >         if (ret < 0) >                 return ret; > >         return IIO_VAL_INT; > } > > To: > >         ret = bmc150_accel_set_power_state(data, false); >         if (ret < 0) >                 goto unlock; > >     ret = IIO_VAL_INT; How do you think about to use the following code variant then? if (!ret) ret = IIO_VAL_INT; > unlock: >         mutex_unlock(&data->mutex); > >         return ret; > } > > And also use the unlock label in the other cases, this is actually > quite a normal pattern. I see little use in a patch like this if there > are still 2 unlock paths after the patch. How long should I wait for corresponding feedback before another small source code adjustment will be appropriate? Regards, Markus