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

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

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

Passing a message back through a strbuf leaves room for such
interpretations for the int return value.

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.

This doesn't help with functions that want to return a pointer (and
NULL on error).  For those, we can use an int output parameter, copy
the kernel's err_ptr trick, or use other sentinels for recoverable
errors that the caller wants to know about.  It hasn't come up in
practice enough for me to be motivated to want to choose between
those.

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

My two cents,
Jonathan