On Wed, Dec 03, 2014 at 01:38:58PM -0800, Jonathan Nieder wrote:
> > What about a struct that has an errno-like value _and_ a fixed-size
> > buffer? I'm thinking something like:
> >
> > struct error {
> > int code;
> > char msg[1024];
> > };
>
> My experience with errno is that it is very hard to anticipate what
> granularity to use with error codes, especially if the error code is
> going to (in some contexts) determine the message printed to the user.
> In practice, it is easier to come up with a message at the error
> detection site and use a generic "something happened" error code
> except in those places where the caller is going to act on specific
> error types that need to be distinguished.
Yeah, I agree. I think error.msg there would be the main use, and
error.code is just gravy. I agree it could simply come back through the
integer return value, too.
> The "< 0 means error" convention gives room to use different exit
> codes for the errors that need to be programmatically distinguished.
> For example, the ref transaction API uses this to distinguish D/F
> conflicts from other errors, while still returning an error message
> through a strbuf output parameter.
Yup. One nice thing about stuffing it into the error struct is that it
saves the caller from declaring a separate variable (assuming they need
"err" anyway to collect the message). I.e.:
struct error err;
if (some_func(fd, &err))
react_to(err.code);
as opposed to:
struct error err;
int ret;
ret = some_func(fd, &err);
if (ret)
react_to(ret);
But I think it's a minority of cases where we care about the specific
value anyway, so it's probably not a big deal either way.
> > If we changed the strbuf to a fixed-size buffer, that would help the
> > allocation issue. Some messages might be truncated, but it seems
> > unlikely in practice. It still loses readability, though.
>
> I don't like the idea of having to choose a size in advance and not
> being able to fit more detailed (perhaps language-specific, and
> including user-specified input) messages lest the buffer overflow.
Is that really a problem in practice? Is a message larger than 1024
characters actually readable? It would have to be broken across many
lines, and then I think we are no longer dealing with an error message,
but some advice (which probably shouldn't go through this mechanism
anyway).
> The allocation of a variable-sized buffer is a small overhead that I
> don't mind incurring on error. In the non-error case, the caller
> doesn't actually have to free the buffer, and if they choose to, the
> overhead incurred is that of free(NULL)'.
I don't care at all about overhead. I care about extra work on the part
of the caller to avoid a leak. It turns:
if (some_func(fd, &err))
return error("%s", err.msg);
into:
if (some_func(fd, &err)) {
error("%s", err.buf);
strbuf_release(&err);
return -1;
}
It may make things a little easier when an intermediate function has
to further munge the message. For example, your 04/14 uses
strbuf_prefixf. But that can also be expressed in a buffer world as:
/* "parent_err" is passed to us from caller, "err" is a local buffer */
if (copy_fd(orig, fd, &err))
return mkerror(&parent_err, "cannot copy '%s': %s", path, err.msg);
Strbufs make more complicated munging possible, but I'd expect prefixing
to be the most common case (except for not munging at all, which I think
is even more common).
I dunno. I just hate to see a system where it's very easy for callers to
introduce memory leaks by forgetting a strbuf_release, or that bogs
callers down with error-handling boilerplate.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html