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

Junio C Hamano
2024-02-06T22:07:39Z

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

> @@ -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.