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
Attachment:
signature.asc
Description: PGP signature
