[PATCH] staging: sunxi: cedrus: centralize cedrus_open exit

[Paul Kocialkowski]

Hi Dan,

On Thu 28 Apr 22, 13:26, Dan Carpenter wrote:
> On Tue, Apr 26, 2022 at 09:39:03AM +0200, Paul Kocialkowski wrote:
> > 
> > > Do-everything gotos are the most bug prone style of error handling.
> > > Imagine the function is trying to do three things.  It fails part way
> > > through.  Now you're trying to undo the second thing which was never
> > > done.  Just moments ago I was just looking at one of these do-everything
> > > bugs where it was using uninitialized memory.
> > 
> > So by that you mean having just one label for all error handling instead
> > of labels for each undo step?
> > 
> 
> Yes.  Don't do that.  If you try to free everything, half the stuff is
> not allocated so you will undo things which have not been done and it
> leads to a bug.
>
> > I've also seen conditionals used in error labels to undo stuff.
> > 
> 
> I don't understand what you're describing?

Typically that would look like:

void *foo = NULL;
void *bar = NULL;

foo = alloc(...);
if (!foo)
	goto single_error;

bar = alloc(...);
if (!bar)
	goto single_error;

...

single_error:
	if (bar)
		free(bar);

	if (foo)
		free(foo);

> > Would you recommend duplicating error cleanup in each error condition?
> > It feels like another set of issue on its own, besides the obvious downside
> > of duplication.
> 
> Let me write a blog about it:
> 
> https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/

Good writeup, thanks! The part about unwinding loops especially, I've always
wondered about the right way to go about it.

Cheers,

Paul 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20220428/8dff5297/attachment-0001.sig>