On Wed, 04 Jul 2018, Matti Vaittinen wrote:

On Wed, Jul 04, 2018 at 11:11:02AM +0100, Lee Jones wrote:

quoted

On Wed, 04 Jul 2018, Matti Vaittinen wrote:

quoted

On Wed, Jul 04, 2018 at 11:39:11AM +0300, Matti Vaittinen wrote:

quoted

On Tue, Jul 03, 2018 at 11:26:00AM +0200, Enric Balletbo Serra wrote:

quoted

Missatge de Matti Vaittinen [off-list ref] 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.

quoted

quoted

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.

quoted

quoted

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)
+{

[...]

quoted

quoted

+}

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