Re: [PATCH] Always check the return value of `repo_read_object_file()`

From: Karthik Nayak <karthik.188@gmail.com>
Date: Feb 5, 2024, 16:10 UTC
Message-ID: <CAOLa=ZQOALZRNqp7dDH0qDWoHwo6_3G8VgVuMbb3C20UdJ4C5A@mail.gmail.com>
In-Reply-To: <pull.1650.git.1707143753726.gitgitgadget@gmail.com>

Hello,

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 11 quoted lines

> diff --git a/bisect.c b/bisect.c
> index f1273c787d9..f75e50c3397 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -158,6 +158,9 @@ static void show_list(const char *debug, int counted, int nr,
>  		const char *subject_start;
>  		int subject_len;
>
> +		if (!buf)
> +			die(_("unable to read %s"), oid_to_hex(&commit->object.oid));
> +

Nit: We know that `repo_read_object_file()` fails on corrupt objects, so
this means that this is only happening when the object doesn't exist. I
wonder if it makes more sense to replace "unable to read %s" which is a
little ambiguous with something like "object %q doesn't exist".

Otherwise, the patch looks good, thanks!