On Wed, Dec 10, 2014 at 11:00:18AM -0800, Junio C Hamano wrote:
> Two potential issues are:
>
> - Callers that ignore errors need to actively ignore errors with
> strbuf_release(&result.msg);
That was my first thought, too. If you want to do anything besides
report_error, you have to deal with the strbuf. But I'd guess that they
often fall into one of two cases:
1. You are just propagating the error to your caller. In which case
it is not _your_ result struct in the first place, and you do not
need to care about deallocating it either way. I.e.:
int some_func(..., struct result *err)
{
if (some_other_func(..., err))
return -1;
...
}
2. You want to ignore the error. I think anybody taking a result
struct (or a strbuf, or whatever) should accept NULL as "do not
bother giving me your message". And the convenience wrappers
should handle that (I think the mkerror example I sent earlier
did), so callees can just do:
return mkerror(err, "whatever: %s", ...);
The remainder could strbuf_release manually, but there would hopefully
not be many of them.
I think I could live with something like that.
> - Callers have to remember that once the report_errors() function
> is called on a "struct result", the struct loses its information.
>
> Neither is insurmountable, but the latter might turn out to be
> cumbersome to work around in some codepaths.
I suspect the message is not that interesting after calling
report_errors(). The "code" flag could remain, as it does not require
deallocation.
> Another alternative may be to have the reporting storage on the side
> of the callee, and have callers that are interested in errors to
> supply a place to store a pointer to it, i.e.
>
> int some_func(..., struct result **errors) {
> static struct result mine;
This makes some_func not reentrant. Which is a hazard both for threaded
code, but also for functions which want to do:
if (some_func(foo, &err_one)) {
/* didn't work? Try an alternative. */
if (!some_func(bar, &err_two))
....
and expect err_one to contain anything useful.
--
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