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

From: Junio C Hamano <[email-masked]>
Date: Tue, 06 Feb 2024 10:42:14 -0800

Patrick Steinhardt <[email-masked]> writes:

>>  			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 think that one is trying to cope with cases where we genuinely do
not have all three variants, not "we thought we had this variant so
we tried to read it into mmfile[i].{ptr,size}, but it turns out that
the object name we had was bad".  So the fallback code for an entirely
different case was masking the breakage the above hunk fixes, and
this being "rerere", it is better to be cautious than sorry.

Thanks for reading the original code carefully.