Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1503719 > unrolled thread

Re: [PATCH 2/7] iio: light: tsl2583: change functions to only have a single exit point

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/7] iio: light: tsl2583: change functions to only have a  single exit point Brian Masney <masneyb@onstation.org> - 2016-10-19 16:40 +0200
    Re: [PATCH 2/7] iio: light: tsl2583: change functions to only have a  single exit point Dan Carpenter <dan.carpenter@oracle.com> - 2016-10-19 18:20 +0200

#1503719 — Re: [PATCH 2/7] iio: light: tsl2583: change functions to only have a single exit point

On Wed, Oct 19, 2016 at 02:08:59PM +0300, Dan Carpenter wrote:
> On Wed, Oct 19, 2016 at 06:32:05AM -0400, Brian Masney wrote:
> > Change the following functions to only have a single exit point:
> > taos_i2c_read(), taos_als_calibrate(), taos_chip_on(),
> > taos_gain_store(), taos_gain_available_show(), taos_luxtable_store()
> > and taos_probe().
> > 
> 
> What's the point of this?  This style of code just makes things more
> complicated and leads to "forgot the error code" bugs.  People think
> that it future proofs the code in case we add locking but I have looked
> into this and it has minimal if any impact at preventing locking bugs.

The reason that I did this was due to the locking that I added later in
the patch series. Each function would only have a single call to
mutex_unlock(). I should have mentioned that in my message.

Brian

[toc] | [next] | [standalone]

#1504071

On Wed, Oct 19, 2016 at 08:38:16AM -0400, Brian Masney wrote:
> On Wed, Oct 19, 2016 at 02:08:59PM +0300, Dan Carpenter wrote:
> > On Wed, Oct 19, 2016 at 06:32:05AM -0400, Brian Masney wrote:
> > > Change the following functions to only have a single exit point:
> > > taos_i2c_read(), taos_als_calibrate(), taos_chip_on(),
> > > taos_gain_store(), taos_gain_available_show(), taos_luxtable_store()
> > > and taos_probe().
> > > 
> > 
> > What's the point of this?  This style of code just makes things more
> > complicated and leads to "forgot the error code" bugs.  People think
> > that it future proofs the code in case we add locking but I have looked
> > into this and it has minimal if any impact at preventing locking bugs.
> 
> The reason that I did this was due to the locking that I added later in
> the patch series. Each function would only have a single call to
> mutex_unlock(). I should have mentioned that in my message.

This kind of trick does not work in real life.  I have looked at it.
In theory, it should work but in real life, empirically, it does not
and it introduces additional new bugs.

People are always looking for a magic bullet but there is no such thing.
The best approach is just to write the simplest, clearest code that you
can.

regards,
dan carpenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web