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

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 6, 2024, 06:51 UTC
Message-ID: <ZcHW_bc6N5umk2G4@tanuki>
In-Reply-To: <pull.1650.git.1707143753726.gitgitgadget@gmail.com>

On Mon, Feb 05, 2024 at 02:35:53PM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>

[snip]

Show 14 quoted lines

> diff --git a/rerere.c b/rerere.c
> index ca7e77ba68c..13c94ded037 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -973,6 +973,9 @@ static int handle_cache(struct index_state *istate,
>  			mmfile[i].ptr = repo_read_object_file(the_repository,
>  							      &ce->oid, &type,
>  							      &size);
> +			if (!mmfile[i].ptr)
> +				die(_("unable to read %s"),
> +				    oid_to_hex(&ce->oid));
>  			mmfile[i].size = size;
>  		}
>  	}

A few lines below this we check whether `mmfile[i].ptr` is `NULL` and replace it with the empty string if so. So this patch here is basically a change in behaviour where we now die instead of falling back to the empty value.

I'm not familiar enough with the code to say whether the old behaviour is intended or not -- it certainly feels somewhat weird to me. But it did leave me wondering and could maybe use some explanation.

Patrick