Re: [PATCH v7 3/3] bisect--helper: `write_terms` shell function in C

From: Pranit Bauva <pranit.bauva@gmail.com>
Date: May 23, 2016, 17:59 UTC
Message-ID: <CAFZEwPNrAj0-8W150Yg1E+7y-y6_XB_=m2BLpaZqAOBRbksPCg@mail.gmail.com>
In-Reply-To: <CAPig+cSOOv9sGGCTD7=5As9Opu3=e9ofkTpFhCzXC9cJPK21Tg@mail.gmail.com>

On Mon, May 23, 2016 at 9:31 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 34 quoted lines

> On Mon, May 23, 2016 at 10:48 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> Reimplement the `write_terms` shell function in C and add a `write-terms`
>> subcommand to `git bisect--helper` to call it from git-bisect.sh . Also
>> remove the subcommand `--check-term-format` as it can now be called from
>> inside the function write_terms() C implementation.
>> [...]
>> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
>> ---
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> @@ -56,18 +58,38 @@ static int check_term_format(const char *term, const char *orig_term)
>> +static int write_terms(const char *bad, const char *good)
>> +{
>> +       FILE *fp;
>> +       int res;
>> +
>> +       if (!strcmp(bad, good))
>> +               return error(_("please use two different terms"));
>> +
>> +       if (check_term_format(bad, "bad") || check_term_format(good, "good"))
>> +               return -1;
>> +
>> +       fp = fopen(git_path_bisect_write_terms(), "w");
>> +       if (!fp)
>> +               return error(_("could not open the file to write terms"));
>
> By adding two pieces of information, you could make this error message
> much more helpful for anyone trying to debug the failure.
> Specifically:
>
> (1) the pathname for which open() failed
> (2) the actual system-level error
>
> For (2), you could use strerror(errno) or, better yet, error_errno()
> which recently graduated to 'master'.

Yes it would definitely be better to make the error message more helpful. I will dig more into this.

>> +       res = fprintf(fp, "%s\n%s\n", bad, good);
>> +       fclose(fp);
>> +       return (res < 0) ? -1 : 0;
>> +}