Re: [PATCH 4/4] Always check `parse_tree*()`'s return value

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 6, 2024, 22:07 UTC
Message-ID: <xmqqh6ilfdqv.fsf@gitster.g>
In-Reply-To: <93abd7000b81bbcd7f5422715b4bbb60c69a7cde.1707212981.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 8 quoted lines

> @@ -707,7 +707,8 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
>  	init_checkout_metadata(&opts.meta, info->refname,
>  			       info->commit ? &info->commit->object.oid : null_oid(),
>  			       NULL);
> -	parse_tree(tree);
> +	if (parse_tree(tree) < 0)
> +		return 128;
>  	init_tree_desc(&tree_desc, tree->buffer, tree->size);

Another thing that caught my attention during today's integration cycle was that this "parse the tree object in variable tree and then call init_tree_desc on it" is a recurring pattern. After the dust settles, we might want to have a helper function perhaps like

    int init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
    {
	if (parse_tree(tree))
		return -1;
	init_tree_desc(desc, tree->buffer, tree->size);
    }

It was a bit irritating as another topic in flight changes the function signature for init_tree_desc(), causing a textual conflict that could be easily avoided.