Re: What's cooking in git.git (Jan 2025, #06; Wed, 22)

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 23, 2025, 01:52 UTC
Message-ID: <xmqq5xm6uwip.fsf@gitster.g>
In-Reply-To: <20250123003613.GA3900660@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Since this hit 'next', it made it into my Coverity runs, producing the
> small fixup below.

Thanks. A greedy me wonders if things like this can be caught by them a bit earlier before they hit 'next', though ;-)

Show 13 quoted lines

> -- >8 --
> Subject: [PATCH] path-walk: drop redundant parse_tree() call
>
> This call to parse_tree() was flagged by Coverity for ignoring the
> return value. But if we look a little further up the function, we can
> see that there is already a call to parse_tree_gently(), and we'll
> return early if that fails. So by this point the tree will always be
> parsed, and the call is redundant.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  path-walk.c | 1 -
>  1 file changed, 1 deletion(-)

Nice way to use extended context to show why the change makes sense.

Will queue.  Thanks.

Show 32 quoted lines

> diff --git a/path-walk.c b/path-walk.c
> index 136ec08fb0..9715a5550e 100644
> --- a/path-walk.c
> +++ b/path-walk.c
> @@ -116,27 +116,26 @@ static int add_tree_entries(struct path_walk_context *ctx,
>  
>  	if (!tree) {
>  		error(_("failed to walk children of tree %s: not found"),
>  		      oid_to_hex(oid));
>  		return -1;
>  	} else if (parse_tree_gently(tree, 1)) {
>  		error("bad tree object %s", oid_to_hex(oid));
>  		return -1;
>  	}
>  
>  	strbuf_addstr(&path, base_path);
>  	base_len = path.len;
>  
> -	parse_tree(tree);
>  	init_tree_desc(&desc, &tree->object.oid, tree->buffer, tree->size);
>  	while (tree_entry(&desc, &entry)) {
>  		struct type_and_oid_list *list;
>  		struct object *o;
>  		/* Not actually true, but we will ignore submodules later. */
>  		enum object_type type = S_ISDIR(entry.mode) ? OBJ_TREE : OBJ_BLOB;
>  
>  		/* Skip submodules. */
>  		if (S_ISGITLINK(entry.mode))
>  			continue;
>  
>  		/* If the caller doesn't want blobs, then don't bother. */
>  		if (!ctx->info->blobs && type == OBJ_BLOB)