git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Improved error handling (Was: [PATCH 1/2] sequencer: factor out rewrite_file())

From
Johannes Sixt <j6t@kdbg.org>
Date
Dec 25, 2017, 10:28 UTC
Message-ID
<16262419-3ce9-13e2-6dbc-2ffcad8327f6@kdbg.org>
In-Reply-To
<20171224145427.GG23648@sigill.intra.peff.net>
Am 24.12.2017 um 15:54 schrieb Jeff King:
Show 26 quoted lines
> On Sat, Nov 18, 2017 at 10:01:45AM +0100, Johannes Sixt wrote:
> 
>>> Yeah, I have mixed feelings on that. I think it does make the control
>>> flow less clear. At the same time, what I found was that handlers like
>>> die/ignore/warn were the thing that gave the most reduction in
>>> complexity in the callers.
>>
>> Would you not consider switching over to C++? With exceptions, you get the
>> error context without cluttering the API. (Did I mention that
>> librarification would become a breeze? Do not die in library routines: not a
>> problem anymore, just catch the exception. die_on_error parameters? Not
>> needed anymore. Not to mention that resource leaks would be much, MUCH
>> simpler to treat.)
> 
> I threw this email on my todo pile since I was traveling when it came,
> but I think it deserves a response (albeit quite late).
> 
> It's been a long while since I've done any serious C++, but I did really
> like the RAII pattern coupled with exceptions. That said, I think it's
> dangerous to do it half-way, and especially to retrofit an existing code
> base. It introduces a whole new control-flow pattern that is invisible
> to the existing code, so you're going to get leaks and variables in
> unexpected states whenever you see an exception.
> 
> I also suspect there'd be a fair bit of in converting the existing code
> to something that actually compiles as C++.

I think I mentioned that I had a version that passed the test suite. It's not pure C++ as it required -fpermissive due to the many implicit void*-to-pointer-to-object conversions (which are disallowed in C++). And, yes, a fair bit of conversion was required on top of that. ;)

Show 5 quoted lines
> So if we were starting the project from scratch and thinking about using
> C++ with RAII and exceptions, sure, that's something I'd entertain[1]
> (and maybe even Linus has softened on his opinion of C++ these days ;) ).
> But at this point, it doesn't seem like the tradeoff for switching is
> there.

Fair enough. I do agree that the tradeoff is not there, in particular, when the major players are more fluent in C than in modern C++.

There is just my usual rant: Why do we have look for resource leaks during review when we could have leak-free code by design? (But Dscho scored a point[*] some time ago: "For every fool-proof system invented, somebody invents a better fool.")

[*] https://public-inbox.org/git/alpine.DEB.2.20.1704281334060.3480@virtualbox/

Previous: Randall S. BeckerNext: Johannes Schindelin
Message 33 of 35 in “sequencer: factor out rewrite_file()”
  1. 1/2 sequencer: factor out rewrite_file()René Scharfe, Oct 31, 2017
  2. 2/2 sequencer: use O_TRUNC to truncate filesRené Scharfe, Oct 31, 2017
  3. Kevin DaudtOct 31, 2017
  4. Johannes SchindelinNov 1, 2017
  5. Kevin DaudtOct 31, 2017
  6. Kevin DaudtNov 1, 2017
  7. Simon RuderichNov 1, 2017
  8. René ScharfeNov 1, 2017
  9. 1/2 wrapper.c: consistently quote filenames in error messagesSimon Ruderich, Nov 1, 2017
  10. Junio C HamanoNov 2, 2017
  11. Junio C HamanoNov 2, 2017
  12. Simon RuderichNov 2, 2017
  13. Junio C HamanoNov 3, 2017
  14. 2/2 sequencer.c: check return value of close() in rewrite_file()Simon Ruderich, Nov 1, 2017
  15. René ScharfeNov 1, 2017
  16. Johannes SchindelinNov 1, 2017
  17. Jeff KingNov 1, 2017
  18. Johannes SchindelinNov 1, 2017
  19. Jeff KingNov 1, 2017
  20. Simon RuderichNov 3, 2017
  21. Junio C HamanoNov 3, 2017
  22. Jeff KingNov 3, 2017
  23. René ScharfeNov 4, 2017
  24. Jeff KingNov 4, 2017
  25. Simon RuderichNov 4, 2017
  26. Jeff KingNov 5, 2017
  27. Simon RuderichNov 6, 2017
  28. Simon RuderichNov 16, 2017
  29. Jeff KingNov 17, 2017
  30. Johannes SixtNov 18, 2017
  31. Jeff KingDec 24, 2017
  32. Randall S. BeckerDec 24, 2017
  33. Johannes SixtDec 25, 2017
  34. Johannes SchindelinNov 3, 2017
  35. Jeff KingNov 3, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.