On Wed, 04 Jul 2018, Matti Vaittinen wrote:
> On Wed, Jul 04, 2018 at 11:11:02AM +0100, Lee Jones wrote:
> > On Wed, 04 Jul 2018, Matti Vaittinen wrote:
> >
> > > On Wed, Jul 04, 2018 at 11:39:11AM +0300, Matti Vaittinen wrote:
> > > > On Tue, Jul 03, 2018 at 11:26:00AM +0200, Enric Balletbo Serra wrote:
> > > > > Missatge de Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx> del
> > > > > dia dv., 29 de juny 2018 a les 11:47:
> > > > >
> > > > > Now that you use devm calls and you don't need to unwind things I
> > > > > think is better to use plain returns. So,
> > > > >
> > > > > return -ENOMEM;
> > > >
> > > > I have never really understood why use of gotos in error handling is
> > > > discouraged.
> >
> > They're not.
> >
> > > > Personally I would always choose single point of exit from
> > > > a function when it is simple enough to achieve (like in this case). I've
> > > > written and fixed way too many functions which leak resources or
> > > > accidentally keep a lock when exiting from error branches. But I know
> > > > many colleagues like you who prefer not to have gotos but in place returns
> > > > instead. So I guess I'll leave the final call on this to the one who is
> > > > maintainer for this code. And it is true there is no things to unwind
> > > > now - which does not mean that next updater won't add such. But as I
> > > > said, I know plenty of people share your view - and even though I rather
> > > > maintain code with only one exit the final call is on subsystem maintainer
> > > > here.
> >
> > Please use gotos in the error path.
> >
> > IMO, it's the nicest way to unwind (as you call it).
>
> I'll keep the gotos but clean other stuff for patch v9 then.
Sounds good.
> > > Actually, If it was completely my call the probe would look something
> > > like this:
> > >
> > > +static int bd71837_i2c_probe(struct i2c_client *i2c,
> > > + const struct i2c_device_id *id)
> > > +{
[...]
> > > +}
> > >
> > > What do you think of this? To my eye it is nice. It keeps single point of
> > > exit and introduces only simple if-statements without the need of curly
> > > brackets. And finally the error prints string works as a comment too.
> > > I've seen bunch of constructs like this on the networking side but I
> > > have no idea if this is frowned on this subsystem =) Oh, and probe abowe
> > > is just to illustrate the idea, I did not even try compiling it yet.
> >
> > That is horrible. I nearly vomited on my keyboard.
>
> Note to self: Never to buy second hand keyboard from Lee =)
That is sound advice.
Not sure I would buy 2nd-hand keyboard from anyone - ewe! :\
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
