[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161019125125.GK4469@mwanda>
Date: Wed, 19 Oct 2016 15:51:25 +0300
From: Dan Carpenter <dan.carpenter@...cle.com>
To: Brian Masney <masneyb@...tation.org>
Cc: devel@...verdev.osuosl.org, lars@...afoo.de,
linux-iio@...r.kernel.org, gregkh@...uxfoundation.org,
linux-kernel@...r.kernel.org, pmeerw@...erw.net, knaack.h@....de,
jic23@...nel.org
Subject: Re: [PATCH 2/7] iio: light: tsl2583: change functions to only have a
single exit point
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
Powered by blists - more mailing lists