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

From: Jeff King <peff@peff.net>
Date: Apr 28, 2015, 06:17 UTC
Message-ID: <20150428061741.GL24580@peff.net>
In-Reply-To: <1430030985-14499-2-git-send-email-erik.elfstrom@gmail.com>

On Sun, Apr 26, 2015 at 08:49:41AM +0200, Erik Elfström wrote:

Show 10 quoted lines

> -extern const char *read_gitfile(const char *path);
> +
> +#define READ_GITFILE_ERR_STAT_FAILED 1
> +#define READ_GITFILE_ERR_NOT_A_FILE 2
> +#define READ_GITFILE_ERR_OPEN_FAILED 3
> +#define READ_GITFILE_ERR_READ_FAILED 4
> +#define READ_GITFILE_ERR_INVALID_FORMAT 5
> +#define READ_GITFILE_ERR_NO_PATH 6
> +#define READ_GITFILE_ERR_NOT_A_REPO 7
> +extern const char *read_gitfile_gently(const char *path, int *return_error_code);

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

[1] The original thread was here:

      http://thread.gmane.org/gmane.comp.version-control.git/259695/focus=260722

    I'm still a little wary of the allocation boilerplate introduced by
    the strbuf approach. But in this case it would not be too bad, I
    think.