From: Stefan Sperling <stsp@stsp.name>
Subject: Re: in diff error message, say what was being diffed
To: James Cook <falsifian@falsifian.org>
Cc: gameoftrees@openbsd.org
Date: Mon, 18 Sep 2023 08:47:31 +0200

On Mon, Sep 18, 2023 at 01:11:22AM +0000, James Cook wrote:
> On Sat, Sep 16, 2023 at 08:16:34PM +0200, Stefan Sperling wrote:
> > Could you use got_error_fmt() instead of snprintf + got_error_msg?
> 
> If I switch to got_error_fmt, then instead of this
> 	got: /dev/null vs /g: diff_atomize_file: Cannot allocate memory
> I get
> 	got: /dev/null vs /g: diff_atomize_file: Cannot allocate memory: see errno
> 
> The trouble is got_error_fmt takes responsibility for appending a string
> describing the error code, which is not useful in this case.

Oh, indeed. Sorry about that.

> One option would be to add a function to the error library that "wraps" an
> error by prepending a string. E.g.
> 
> const struct got_error *got_error_prepend_msg(
>     const struct got_error*, const char *);
> 
> I don't know if there are other places where that would come in handy.

There is another function, called got_error_from_errno_fmt().
Would that one work?
In any case, I would rather tweak the existing error interfaces to
match our needs than add even more variants.

> To be precise, the new diff I tried that gave the extra "see errno" was:
> 
> --BEGIN--
> --- lib/diff.c
> +++ lib/diff.c
> @@ -378,8 +378,12 @@ diff_blob_file(struct got_diffreg_result **resultp,
>         err = got_diffreg(&result, f1, f2, diff_algo, ignore_whitespace,
>             force_text_diff);
> -       if (err)
> -               goto done;
> +       if (err) {
> +               err = got_error_fmt(err->code, "%s vs %s: %s",
> +                           label1 ? label1 : idstr1,
> +                           f2_exists ? label2 : "/dev/null", err->msg);
> +                goto done;
> +       }
>         if (outfile) {
>                 err = got_diffreg_output(NULL, NULL, result,
> --END--