Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Dec 10, 2014, 17:02 UTC
Message-ID: <54887CB7.4000603@alum.mit.edu>
In-Reply-To: <20141204075920.GA27142@peff.net>

On 12/04/2014 08:59 AM, Jeff King wrote:

Show 19 quoted lines

> On Wed, Dec 03, 2014 at 01:38:58PM -0800, Jonathan Nieder wrote:
>> 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;
>   }

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.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu