lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 25 Oct 2017 20:07:48 +0200
From:   SF Markus Elfring <elfring@...rs.sourceforge.net>
To:     Hans de Goede <hdegoede@...hat.com>, linux-iio@...r.kernel.org
Cc:     Hartmut Knaack <knaack.h@....de>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: iio/accel/bmc150: Improve unlocking of a mutex in two functions

> 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

Powered by blists - more mailing lists