Michael Haggerty <mhag...@alum.mit.edu> writes:

> What if we go in the direction not of less infrastructure, but a little
> bit more? Like
>
>       struct result {
>               int code;
>               struct strbuf msg;
>       };
>
>       int report_errors(struct result *result)
>       {
>               int code = result->code;
>               if (code) {
>                       error(result->msg.buf);
>               }
>               result->code = 0;
>               strbuf_release(result->msg);
>               return code; /* or alternatively (code ? -1 : 0) */
>       }
>
>       int report_warnings(struct result *result)
>       {
>               ...
>       }
>
>       int report_with_prefix(struct result *result, const char *fmt, ...)
>       {
>               ...
>       }
>
> Then a caller could look pretty much like before:
>
>       struct result result = RESULT_INIT;
>
>       if (some_func(fd, &result))
>               return report_errors(&result);
>
> Other callers might not even bother to check the return value of the
> function, relying instead on result.code via process_error():
>
>       char *ptr = some_func(fd, &result);
>       if (report_errors(&result))
>               return -1;
>
> If the result code is considered superfluous, we could use naked strbufs
> and use msg.len as the indicator that there was an error.

Two potential issues are:

 - Callers that ignore errors need to actively ignore errors with
   strbuf_release(&result.msg);

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

Other than that, I think it is OK.

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;

                clear_result(&mine);
                ... do its thing ...
                if (... error detected ...) {
                        mine.code = E_SOMEFUNC;
                        strbuf_addstr(&mine.msg, "some_func: foobared");
                }

                if (errors)
                        *errors = &mine;
                return mine.code;
        }

And a caller would do this instead:

        struct result *result = NULL;

        if (some_func(fd, &result))
                return report_errors(result);

where report_errors() and friends will only peek but not clear the
result reporting storage.  The clear_result() helper would trivially
be:

        void clear_result(struct result *result) {
                result->code = 0;
                strbuf_release(&result->msg);
        }

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to