Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

["erik elfström" <erik.elfstrom@gmail.com>]

On Tue, Apr 28, 2015 at 8:17 AM, Jeff King <peff@peff.net> wrote:
>
> There was a discussion not too long ago on strategies for returning
> errors, and one of the suggestions was to return an "error strbuf"
> rather than a code[1]. That's less flexible, as the caller can't react
> differently based on the type of error. But for cases like this, where
> the only fate for the code is to get converted back into a message,
> it can reduce the boilerplate.
>
> What you have here is OK to me, and I don't want to hold up your patch
> series in a flamewar about error-reporting techniques. But I think it's
> an interesting case study.
>
> -Peff

Thanks. I haven't had time to look through that thread yet, I'll try
to get to that later.

My initial reaction is a bit skeptical though. For this case we
currently don't want any error reporting, the NULL return is
sufficient and even allocating and sending in the int* is pure noise.
Allocating and releasing a strbuf seems like a lot more overhead for
this type of caller? The one other potential candidate caller for
read_gitfile_gently that I have seen (clone.c:get_repo_path) don't
want any error code or message either as far as i can tell.

Also if it turns out that we actually need to treat the "file too
large" error differently in clean (as discussed in thread on the file
size check) then we can no longer communicate that back using the
strbuf interface.

Overall it seems like a less attractive solution to me but I can be
persuaded if there is more support expressed for the strbuf direction.

/Erik