From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751901AbdJYSIO (ORCPT ); Wed, 25 Oct 2017 14:08:14 -0400 Received: from mout.web.de ([212.227.15.4]:49808 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751702AbdJYSIL (ORCPT ); Wed, 25 Oct 2017 14:08:11 -0400 Subject: Re: 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> <6a2c614a-4d0d-f2d2-1689-4c67a2fc3eea@redhat.com> <7a76d804-797c-8a7f-a755-9e42f9157287@users.sourceforge.net> <26467a3c-00e7-7358-5087-bf35469feb06@redhat.com> From: SF Markus Elfring Message-ID: <12013256-ad6a-86ed-9bc1-e5d868189914@users.sourceforge.net> Date: Wed, 25 Oct 2017 20:07:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <26467a3c-00e7-7358-5087-bf35469feb06@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:gjUPRA+MTfanX5JGLmPtQUgAZcIxxzmrXaEsamstO1YT3c1bCog X1/5HtZCMXQMHUaabBeVXZjXM0zkrBAM9uuFnJ0hZIfFeNBUq01ZA6Wl9OjkUCKMfsxoUOJ jFCw7bhcgHTtBMp5iaAE1/EMOoTbm4ijigDWADzwpJSfrPC6VKtB8ebRqejvQxJoM1Vm/kj Ax+fMlejzOGuJeF0GEi+Q== X-UI-Out-Filterresults: notjunk:1;V01:K0:8yW9BjEEb2s=:97gwq+no7tKg9cVS/OaBKs qOOwlnnLsVQybh7YbwDMT3DrBxsr9knpXgfpiqxVlG3hcp0p4GHcP7yUcqM6XTHseHVPACmRp hm3vguM2hvBtF9FYl9Wbeidz+MS+P8uxD9rb+ulzGPf83grstoa5DV9yxnUxFX7sYqGiSx0Fg ziQjwoQW8vVGGwk5vto5QuRatKat4JliRbKsXNvMoVObed5Rsf4AkOIUi6xGqTcwXtkzZ2cnl Ghav+OwmlDl3xZC9dZ04MPBNGyZcVRUkg2u2QR4oSgzdJdMGHllie1RWNByRk2399OJBUTQ/4 qh/f5yGkDk2cUO52U+/WJ93ReCDZZMM1ENmXdiVYG/RZ3pdJKfeylLrjQ+q/oWd4fyERBjvLg mQzi+LK6qshJDv+Qy2XozCtOsSCxZ1cMq1mo+jeOm8HbwX4ReB7KtcP4jT4tNhk6WZf9wW9ao dyFvyjNe396DHPOqKvt/iJiCNF4qDhWPznDSOOJlJ8aSdQjhXL1jHS4gYq2cyPIVct76iJdvX HgTXJnQCNxIs5GKgeN9YmznDoakxXR49k9i/WKeB3LYtloWmkKg62bbtHprV0SWyIcqcmXOXh tFiuS/g1Lh1B9LBo/1it9Byo1k6Qy8pUmghTKpuTbuxQyJXCQDala5qFoLAoLSDdMdMjp8gx0 obUKXlv2yO1kp64lvVegV8NLOvNNAUbNdB+4cSTWEl8IadOBqxtHD1UTX2ZYK6OUpjzn7h5km EhNRNU0uD3EiE8NQk/l9Y3sTXRCrPBebszJCJQxAXTDz3G4yplNEiP6k5qiaz0PXtk9WSVaqu SgORGk7rO4+C81aQ/fx+C5IhZQRwRlNJr8x+nEVkOAE9wm0RTk= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > What you are suggesting breaks this pattern I might be looking for an other balance between involved implementation details after your constructive feedback for my first approach in this software module. > (not using a goto in the last if (err) case) I would find it nice when a bit more code reduction is feasible. > which makes the code harder to read and makes things harder > (and potentially leads to introducing bugs) when > a step4() gets added. There is a choice between conservative adjustments and progressive software refactoring where both directions can lead to similar development risks. >>> because that way the error handling is consistent between all steps >>> and if another step is later added at the end, the last step will >>> not require modification. Such a view might express a change resistance. >> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/iio/accel/kxcjk-1013.c?id=1540d0106bcbc4e52013d759a0a0752ae7b4a09d#n760 > > So I just checked this one, Thanks for your interest. > this one is tricky because the lock is taking inside > a switch-case and doing gotos inside the case is not pretty. I imagine that I would like to use scoped lock objects in affected source files. (Other programming languages support such synchronisation constructs directly.) > Basically I believe there is no one-size fits all solution > here and refactoring like this may introduce bugs, so one > needs to weight the amount of work + regression risk vs > the benefits of the code being cleaner going forward. It seems that our software development discussion can be continued in a constructive way then. Regards, Markus