Re: [PATCH 1/2] sequencer: factor out rewrite_file()

Simon Ruderich [off-list ref] writes:

I tried looking into this by adding a new write_file_buf_gently()
(or maybe renaming write_file_buf to write_file_buf_or_die) and
using it from write_file_buf() but I don't know the proper way to
handle the error-case in write_file_buf(). Just calling
die("write_file_buf") feels ugly, as the real error was already
printed on screen by error_errno() and I didn't find any function
to just exit without writing a message (which still respects
die_routine). Suggestions welcome.

How about *not* printing the error at the place where you notice the
error, and instead return an error code to the caller to be noticed
which dies with an error message?

On Fri, Nov 03, 2017 at 10:44:08PM +0900, Junio C Hamano wrote:

Simon Ruderich [off-list ref] writes:

quoted

I tried looking into this by adding a new write_file_buf_gently()
(or maybe renaming write_file_buf to write_file_buf_or_die) and
using it from write_file_buf() but I don't know the proper way to
handle the error-case in write_file_buf(). Just calling
die("write_file_buf") feels ugly, as the real error was already
printed on screen by error_errno() and I didn't find any function
to just exit without writing a message (which still respects
die_routine). Suggestions welcome.

How about *not* printing the error at the place where you notice the
error, and instead return an error code to the caller to be noticed
which dies with an error message?

That ends up giving less-specific errors. It might be an OK tradeoff
here.

I think we've been gravitating towards error strbufs, which would make
it something like:

diff --git a/wrapper.c b/wrapper.c
index 61aba0b5c1..08eb5d1cb8 100644
--- a/wrapper.c
+++ b/wrapper.c

@@ -649,13 +649,34 @@ int xsnprintf(char *dst, size_t max, const char *fmt, ...)
 	return len;
 }
 
+int write_file_buf_gently(const char *path, const char *buf, size_t len,
+			  struct strbuf *err)
+{
+	int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+	if (fd < 0) {
+		strbuf_addf(err, _("could not open '%s' for writing: %s"),
+			    path, strerror(errno));
+		return -1;
+	}
+	if (write_in_full(fd, buf, len) < 0) {
+		strbuf_addf(err, _("could not write to %s: %s"),
+			    path, strerror(errno));
+		close(fd);
+		return -1;
+	}
+	if (close(fd)) {
+		strbuf_addf(err, _("could not close %s: %s"),
+			    path, strerror(errno));
+		return -1;
+	}
+	return 0;
+}
+
 void write_file_buf(const char *path, const char *buf, size_t len)
 {
-	int fd = xopen(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
-	if (write_in_full(fd, buf, len) < 0)
-		die_errno(_("could not write to %s"), path);
-	if (close(fd))
-		die_errno(_("could not close %s"), path);
+	struct strbuf err = STRBUF_INIT;
+	if (write_file_buf_gently(path, buf, len, &err) < 0)
+		die("%s", err.buf);
 }
 
 void write_file(const char *path, const char *fmt, ...)

I'm not excited that the amount of error-handling code is now double the
amount of code that actually does something useful. Maybe this function
simply isn't large/complex enough to merit flexible error handling, and
we should simply go with René's original near-duplicate.

OTOH, if we went all-in on flexible error handling contexts, you could
imagine this function becoming:

  void write_file_buf(const char *path, const char *buf, size_t len,
                      struct error_context *err)
  {
	int fd = xopen(path, err, O_WRONLY | O_CREAT | O_TRUNC, 0666);
	if (fd < 0)
		return -1;
	if (write_in_full(fd, buf, len, err) < 0)
		return -1;
	if (xclose(fd, err) < 0)
		return -1;
	return 0;
  }

Kind of gross, in that we're adding a layer on top of all system calls.
But if used consistently, it makes error-reporting a lot more pleasant,
and makes all of our "whoops, we forgot to save errno" bugs go away.

-Peff

Am 03.11.2017 um 20:13 schrieb Jeff King:

On Fri, Nov 03, 2017 at 10:44:08PM +0900, Junio C Hamano wrote:

quoted

Simon Ruderich [off-list ref] writes:

quoted

I tried looking into this by adding a new write_file_buf_gently()
(or maybe renaming write_file_buf to write_file_buf_or_die) and
using it from write_file_buf() but I don't know the proper way to
handle the error-case in write_file_buf(). Just calling
die("write_file_buf") feels ugly, as the real error was already
printed on screen by error_errno() and I didn't find any function
to just exit without writing a message (which still respects
die_routine). Suggestions welcome.

How about *not* printing the error at the place where you notice the
error, and instead return an error code to the caller to be noticed
which dies with an error message?

That ends up giving less-specific errors.

Not necessarily.  Function could return different codes for different
errors, e.g. -1 for an open(2) error and -2 for a write(2) error, and
the caller could use that to select the message to show.

Basically all of the messages in wrapper.c consist of some text mixed
with the affected path path and a strerror(3) string, so they're
compatible in that way.  A single function (get_path_error_format()?)
could thus be used and callers would be able to combine its result with
die(), error(), or warning().

René

On Sat, Nov 04, 2017 at 10:05:43AM +0100, René Scharfe wrote:

quoted

quoted

How about *not* printing the error at the place where you notice the
error, and instead return an error code to the caller to be noticed
which dies with an error message?

That ends up giving less-specific errors.

Not necessarily.  Function could return different codes for different
errors, e.g. -1 for an open(2) error and -2 for a write(2) error, and
the caller could use that to select the message to show.

Basically all of the messages in wrapper.c consist of some text mixed
with the affected path path and a strerror(3) string, so they're
compatible in that way.  A single function (get_path_error_format()?)
could thus be used and callers would be able to combine its result with
die(), error(), or warning().

I think we've had this discussion before, a while ago. Yes, returning an
integer error code is nice because you don't have pass in an extra
parameter. But I think there are two pitfalls:

  1. Integers may not be descriptive enough to cover all cases, which is
     how we ended up with the strbuf-passing strategy in the ref code.
     Certainly you could add an integer for every possible bespoke
     message, but then I'm not sure it's buying that much over having
     the function simply fill in a strbuf.

  2. For complex functions there may be multiple errors that need to
     stack. I think the refs code has cases like this, where a syscall
     fails, which causes a fundamental ref operation to fail, which
     causes a higher-level operation to fail. It's only the caller of
     the higher-level operation that knows how to report the error.

Certainly an integer error code would work for _this_ function, but I'd
rather see us grow towards consistent error handling.

-Peff

On Fri, Nov 03, 2017 at 03:13:10PM -0400, Jeff King wrote:

I think we've been gravitating towards error strbufs, which would make
it something like:

I like this approach to store the error in a separate variable
and let the caller handle it. This provides proper error messages
and is cleaner than printing the error on the error site (what
error_errno does).

However I wouldn't use strbuf directly and instead add a new
struct error which provides a small set of helper functions.
Using a separate type also makes it clear to the reader that is
not a normal string and is more extendable in the future.

I'm not excited that the amount of error-handling code is now double the
amount of code that actually does something useful. Maybe this function
simply isn't large/complex enough to merit flexible error handling, and
we should simply go with René's original near-duplicate.

A separate struct (and helper functions) would help in this case
and could look like this, which is almost equal (in code size) to
the original solution using error_errno:

    int write_file_buf_gently2(const char *path, const char *buf, size_t len, struct error *err)
    {
            int rc = 0;
            int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
            if (fd < 0)
                    return error_addf_errno(err, _("could not open '%s' for writing"), path);
            if (write_in_full(fd, buf, len) < 0)
                    rc = error_addf_errno(err, _("could not write to '%s'"), path);
            if (close(fd) && !rc)
                    rc = error_addf_errno(err, _("could not close '%s'"), path);
            return rc;
    }

(I didn't touch write_in_full here, but it could also take the
err and then the code would get a little shorter, however would
lose the "path" information, but see below.)

And in the caller:

    void write_file_buf(const char *path, const char *buf, size_t len)
    {
            struct error err = ERROR_INIT;
            if (write_file_buf_gently2(path, buf, len, &err) < 0)
                    error_die(&err);
    }

For now struct error just contains the strbuf, but one could add
the call location (by using a macro for error_addf_errno) or the
original errno or more information in the future.

error_addf_errno() could also prepend the error the buffer so
that the caller can add more information if necessary and we get
something like: "failed to write file 'foo': write failed: errno
text" in the write_file_buf case (the first error string is from
write_file_buf_gently2, the second from write_in_full). However
I'm not sure how well this works with translations.

We could also store the error condition in the error struct and
don't use the return value to indicate and error like this:

    void write_file_buf(const char *path, const char *buf, size_t len)
    {
            struct error err = ERROR_INIT;
            write_file_buf_gently2(path, buf, len, &err);
            if (err.error)
                    error_die(&err);
    }

OTOH, if we went all-in on flexible error handling contexts, you could
imagine this function becoming:

  void write_file_buf(const char *path, const char *buf, size_t len,
                      struct error_context *err)
  {
	int fd = xopen(path, err, O_WRONLY | O_CREAT | O_TRUNC, 0666);
	if (fd < 0)
		return -1;
	if (write_in_full(fd, buf, len, err) < 0)
		return -1;
	if (xclose(fd, err) < 0)
		return -1;
	return 0;
  }

This looks interesting as well, but it misses the feature of
custom error messages which is really useful.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

On Sat, Nov 04, 2017 at 07:36:43PM +0100, Simon Ruderich wrote:

On Fri, Nov 03, 2017 at 03:13:10PM -0400, Jeff King wrote:

quoted

I think we've been gravitating towards error strbufs, which would make
it something like:

I like this approach to store the error in a separate variable
and let the caller handle it. This provides proper error messages
and is cleaner than printing the error on the error site (what
error_errno does).

However I wouldn't use strbuf directly and instead add a new
struct error which provides a small set of helper functions.
Using a separate type also makes it clear to the reader that is
not a normal string and is more extendable in the future.

Yes, I think what you've written here (and below) is quite close to the
error_context patches I linked elsewhere in the thread. In other
words, I think it's a sane approach.

We could also store the error condition in the error struct and
don't use the return value to indicate and error like this:

    void write_file_buf(const char *path, const char *buf, size_t len)
    {
            struct error err = ERROR_INIT;
            write_file_buf_gently2(path, buf, len, &err);
            if (err.error)
                    error_die(&err);
    }

I agree it might be nice for the error context to have a positive "there
was an error" flag. It's probably worth making it redundant with the
return code, though, so callers can use whichever style is most
convenient for them.

quoted

OTOH, if we went all-in on flexible error handling contexts, you could
imagine this function becoming:

  void write_file_buf(const char *path, const char *buf, size_t len,
                      struct error_context *err)
  {
	int fd = xopen(path, err, O_WRONLY | O_CREAT | O_TRUNC, 0666);
	if (fd < 0)
		return -1;
	if (write_in_full(fd, buf, len, err) < 0)
		return -1;
	if (xclose(fd, err) < 0)
		return -1;
	return 0;
  }

This looks interesting as well, but it misses the feature of
custom error messages which is really useful.

Right, I didn't think that example through. The functions after the
open() don't have enough information to make a good message.

-Peff

On Sat, Nov 04, 2017 at 10:07:00PM -0400, Jeff King wrote:

Yes, I think what you've written here (and below) is quite close to the
error_context patches I linked elsewhere in the thread. In other
words, I think it's a sane approach.

In contrast to error_context I'd like to keep all exiting
behavior (die, ignore, etc.) in the hand of the caller and not
use any callbacks as that makes the control flow much harder to
follow.

I agree it might be nice for the error context to have a positive "there
was an error" flag. It's probably worth making it redundant with the
return code, though, so callers can use whichever style is most
convenient for them.

Agreed.

Regarding the API, should it be allowed to pass NULL as error
pointer to request no additional error handling or should the
error functions panic on NULL? Allowing NULL makes partial
conversions possible (e.g. for write_in_full) where old callers
just pass NULL and check the return values and converted callers
can use the error struct.

How should translations get handled? Appending ": %s" for
strerror(errno) might be problematic. Same goes for "outer
message: inner message" where the helper function just inserts ":
" between the messages. Is _("%s: %s") (with appropriate
translator comments) enough to handle these cases?

Suggestions how to name the struct and the corresponding
functions? My initial idea was struct error and to use error_ as
prefix, but I'm not sure if struct error is too broad and may
introduce conflicts with system headers. Also error_ is a little
long and could be shorted to just err_ but I don't know if that's
clear enough. The error_ prefix doesn't conflict with many git
functions, but there are some in usage.c (error_errno, error,
error_routine).

And as general question, is this approach to error handling
something we should pursue or are there objections? If there's
consensus that this might be a good idea I'll look into
converting some parts of the git code (maybe refs.c) to see how
it pans out.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

On Mon, Nov 06, 2017 at 05:13:15PM +0100, Simon Ruderich wrote:

On Sat, Nov 04, 2017 at 10:07:00PM -0400, Jeff King wrote:

quoted

Yes, I think what you've written here (and below) is quite close to the
error_context patches I linked elsewhere in the thread. In other
words, I think it's a sane approach.

In contrast to error_context I'd like to keep all exiting
behavior (die, ignore, etc.) in the hand of the caller and not
use any callbacks as that makes the control flow much harder to
follow.

quoted

I agree it might be nice for the error context to have a positive "there
was an error" flag. It's probably worth making it redundant with the
return code, though, so callers can use whichever style is most
convenient for them.

Agreed.

Regarding the API, should it be allowed to pass NULL as error
pointer to request no additional error handling or should the
error functions panic on NULL? Allowing NULL makes partial
conversions possible (e.g. for write_in_full) where old callers
just pass NULL and check the return values and converted callers
can use the error struct.

How should translations get handled? Appending ": %s" for
strerror(errno) might be problematic. Same goes for "outer
message: inner message" where the helper function just inserts ":
" between the messages. Is _("%s: %s") (with appropriate
translator comments) enough to handle these cases?

Suggestions how to name the struct and the corresponding
functions? My initial idea was struct error and to use error_ as
prefix, but I'm not sure if struct error is too broad and may
introduce conflicts with system headers. Also error_ is a little
long and could be shorted to just err_ but I don't know if that's
clear enough. The error_ prefix doesn't conflict with many git
functions, but there are some in usage.c (error_errno, error,
error_routine).

And as general question, is this approach to error handling
something we should pursue or are there objections? If there's
consensus that this might be a good idea I'll look into
converting some parts of the git code (maybe refs.c) to see how
it pans out.

Any comments?

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

On Mon, Nov 06, 2017 at 05:13:15PM +0100, Simon Ruderich wrote:

On Sat, Nov 04, 2017 at 10:07:00PM -0400, Jeff King wrote:

quoted

Yes, I think what you've written here (and below) is quite close to the
error_context patches I linked elsewhere in the thread. In other
words, I think it's a sane approach.

In contrast to error_context I'd like to keep all exiting
behavior (die, ignore, etc.) in the hand of the caller and not
use any callbacks as that makes the control flow much harder to
follow.

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.

Regarding the API, should it be allowed to pass NULL as error
pointer to request no additional error handling or should the
error functions panic on NULL? Allowing NULL makes partial
conversions possible (e.g. for write_in_full) where old callers
just pass NULL and check the return values and converted callers
can use the error struct.

I think it's probably better to be explicit, and pass some "noop" error
handling struct. We'll have to be adding parameters to functions to
handle this anyway, so I don't think there's much opportunity for having
NULL as a fallback for partial conversions.

How should translations get handled? Appending ": %s" for
strerror(errno) might be problematic. Same goes for "outer
message: inner message" where the helper function just inserts ":
" between the messages. Is _("%s: %s") (with appropriate
translator comments) enough to handle these cases?

I don't have a real opinion, not having done much translation myself. I
will say that the existing die_errno(), error_errno(), etc just use "%s:
%s", without even allowing for translation (see fmt_with_err in
usage.c). I'm sure that probably sucks for RTL languages, but I think
it would be fine to punt on it for now.

Suggestions how to name the struct and the corresponding
functions? My initial idea was struct error and to use error_ as
prefix, but I'm not sure if struct error is too broad and may
introduce conflicts with system headers. Also error_ is a little
long and could be shorted to just err_ but I don't know if that's
clear enough. The error_ prefix doesn't conflict with many git
functions, but there are some in usage.c (error_errno, error,
error_routine).

In my experiments[1] I called the types error_*, and then generally used
"err" as a local variable when necessary. Variants on that seem fine to
me, but yeah, you have to avoid conflicting with error(). We _could_
rename that, but it would be a pretty invasive patch.

And as general question, is this approach to error handling
something we should pursue or are there objections? If there's
consensus that this might be a good idea I'll look into
converting some parts of the git code (maybe refs.c) to see how
it pans out.

I dunno. I kind of like the idea, but if the only error context is one
that adds to strbufs, I don't know that it's buying us much over the
status quo (which is passing around strbufs). It's a little more
explicit, I guess.

Other list regulars besides me seem mostly quiet on the subject.

-Peff

[1] This is the jk/error-context-wip branch of https://github.com/peff/git.
    I can't remember if I mentioned that before.

Am 17.11.2017 um 23:33 schrieb Jeff King:

On Mon, Nov 06, 2017 at 05:13:15PM +0100, Simon Ruderich wrote:

quoted

On Sat, Nov 04, 2017 at 10:07:00PM -0400, Jeff King wrote:

quoted

Yes, I think what you've written here (and below) is quite close to the
error_context patches I linked elsewhere in the thread. In other
words, I think it's a sane approach.

In contrast to error_context I'd like to keep all exiting
behavior (die, ignore, etc.) in the hand of the caller and not
use any callbacks as that makes the control flow much harder to
follow.

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.)

-- Hannes

On Sat, Nov 18, 2017 at 10:01:45AM +0100, Johannes Sixt wrote:

quoted

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++.

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.

-Peff

[1] I'd also consider Rust, though I'm not too experienced with it
    myself.

On December 24, 2017 9:54 AM, Jeff King wrote:

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

On Sat, Nov 18, 2017 at 10:01:45AM +0100, Johannes Sixt wrote:

quoted

quoted

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++.

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.

While I'm a huge fan of OO, you really need a solid justification for going there, and a good study of your target audience for Open Source C++. My comments are based on porting experience outside of Linux/Windows:

1. Conversion to C++ just to pick up exceptions is a lot like "One does not simply walk to Mordor", as Peff hinted at above.
2. Moving to C++ generally involves a **complete** redesign. While Command Patterns (and and...)  may be very helpful in one level, the current git code base is very procedural in nature.
3. From a porting perspective, applications written in with C++ generally (there are exceptions) are significantly harder than C. The POSIX APIs are older and more broadly supported/emulated than what is available in C++. Once you start getting into "my favourite C++ library is...", or "version2 or version3", or smart pointers vs. scope allocation, things get pretty argumentative. It is (arguably) much easier to disable a section of code that won't function on a platform in C without having to rework an OO model, making subsequent merges pretty much impossible and the port unsustainable. That is unless the overall design really factors in platform differences right into the OO model from the beginning of incubation.
4. I really hate making these points because I am an OO "fanspert", just not when doing portable code. Even in java, which is more port-stable than C++ (arguably, but in my experience), you tend to hit platform library differences than can invalidate ports.

My take is "oh my please don't go there" for the git project, for a component that has become/is becoming required core infrastructure for so many platforms.

With Respect,
Randall

-- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400)/NonStop(211288444200000000)
-- In my real life, I talk too much.

Am 24.12.2017 um 15:54 schrieb Jeff King:

On Sat, Nov 18, 2017 at 10:01:45AM +0100, Johannes Sixt wrote:

quoted

quoted

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. ;)

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/