Re: [PATCH 0/2] fixing parse_object() check for type mismatch

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Nov 21 2022, Jeff King wrote:

> On Fri, Nov 18, 2022 at 02:05:04PM -0500, Taylor Blau wrote:
>
>> On Thu, Nov 17, 2022 at 05:37:29PM -0500, Jeff King wrote:
>> > I'm adding Taylor to the cc as the author of t6102, when we were
>> > tracking down all of these "oops, it's not really a blob" cases. This
>> > fixes one of the lingering cases from that test script.
>> >
>> >   [1/2]: parse_object(): drop extra "has" check before checking object type
>> >   [2/2]: parse_object(): check on-disk type of suspected blob
>> >
>> >  object.c                               | 5 ++---
>> >  t/t6102-rev-list-unexpected-objects.sh | 4 ++--
>> >  2 files changed, 4 insertions(+), 5 deletions(-)
>> 
>> A blast from the past :-).
>> 
>> I took a careful look at both of these patches and they looked good to
>> me, so let's start merging them down.
>
> I saw this hit 'next', but I think Ævar's simplification suggestion is
> worth taking. So here is a patch on top to do so (the original branch is
> jk/parse-object-type-mismatch for the benefit of any newly-returned
> maintainers).
>
> I was going to do a "helped-by", but since the only thing in the patch
> is the suggested change, I just handed over authorship. :)
>
> I didn't forge a signoff, and I think mine is sufficient under DCO's
> part (b), but Ævar please indicate if that's OK.

This looks good to me, thanks for following up. In case my SOB is needed
feel free to add it, but it's fine without that too as far as I'm
concerned.

> -- >8 --
> From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Subject: [PATCH] parse_object(): simplify blob conditional
>
> Commit 8db2dad7a0 (parse_object(): check on-disk type of suspected blob,
> 2022-11-17) simplified the conditional for checking if we might have a
> blob. But we can simplify it further. In:
>
>   !obj || (obj && obj->type == OBJ_BLOB)
>
> the short-circuit "OR" means "obj" will always be true on the right-hand
> side. The compiler almost certainly optimized that out anyway, but
> dropping it makes the conditional easier to understand for humans.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/object.c b/object.c
> index fad1a5af4a..682b852a46 100644
> --- a/object.c
> +++ b/object.c
> @@ -286,7 +286,7 @@ struct object *parse_object_with_flags(struct repository *r,
>  			return &commit->object;
>  	}
>  
> -	if ((!obj || (obj && obj->type == OBJ_BLOB)) &&
> +	if ((!obj || obj->type == OBJ_BLOB) &&
>  	    oid_object_info(r, oid, NULL) == OBJ_BLOB) {
>  		if (!skip_hash && stream_object_signature(r, repl) < 0) {
>  			error(_("hash mismatch %s"), oid_to_hex(oid));