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