Re: iio/accel/bmc150: Improve unlocking of a mutex in two functions

From: SF Markus Elfring
Date: Wed Oct 25 2017 - 14:08:18 EST


> 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