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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 6, 2024, 22:02 UTC
Message-ID: <xmqqplx9fdyk.fsf@gitster.g>
In-Reply-To: <pull.1650.git.1707143753726.gitgitgadget@gmail.com>

Show 13 quoted lines

> diff --git a/builtin/grep.c b/builtin/grep.c
> index c8e33f97755..982bcfc4b1d 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -571,6 +571,8 @@ static int grep_cache(struct grep_opt *opt,
>  
>  			data = repo_read_object_file(the_repository, &ce->oid,
>  						     &type, &size);
> +			if (!data)
> +				die(_("unable to read tree %s"), oid_to_hex(&ce->oid));
>  			init_tree_desc(&tree, data, size);
>  
>  			hit |= grep_tree(opt, pathspec, &tree, &name, 0, 0);

This caught my attention during today's integration cycle. Checking nullness for data certainly is an improvement, but shouldn't we be checking type as well to make sure it is a tree and not a random tree-ish or even blob?