Re: [PATCH 4/7] grep: read submodule entry with explicit repo
From: Emily Shaffer <hidden>
Date: 2021-08-11 21:44:59
On Tue, Aug 10, 2021 at 11:28:42AM -0700, Jonathan Tan wrote:
Replace an existing parse_object_or_die() call (which implicitly works on the_repository) with a function call that allows a repository to be passed in. There is no such direct equivalent to parse_object_or_die(), but we only need the type of the object, so replace with oid_object_info().
Always exciting to see less implicit use of the_repository ;)
quoted hunk ↗ jump to hunk
Signed-off-by: Jonathan Tan <redacted> --- builtin/grep.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)diff --git a/builtin/grep.c b/builtin/grep.c index e454335e9d..9e61c7c993 100644 --- a/builtin/grep.c +++ b/builtin/grep.c@@ -457,27 +457,27 @@ static int grep_submodule(struct grep_opt *opt, subopt.repo = &subrepo; if (oid) { - struct object *object; + enum object_type object_type; struct tree_desc tree; void *data; unsigned long size; struct strbuf base = STRBUF_INIT; obj_read_lock(); - object = parse_object_or_die(oid, NULL); + object_type = oid_object_info(&subrepo, oid, NULL);
One thing I wonder is whether we are missing out on some error conditions we used to get with parse_object_or_die() by using oid_object_info() instead. Do we need to be more defensive in investigating 'oid' before calling that helper, now?
obj_read_unlock(); data = read_object_with_reference(&subrepo, - &object->oid, tree_type, + oid, tree_type,
And a handful of instances where we were using object->oid instead of the oid we were already passed. Ok.
&size, NULL);
if (!data)
- die(_("unable to read tree (%s)"), oid_to_hex(&object->oid));
+ die(_("unable to read tree (%s)"), oid_to_hex(oid));
strbuf_addstr(&base, filename);
strbuf_addch(&base, '/');
init_tree_desc(&tree, data, size);
hit = grep_tree(&subopt, pathspec, &tree, &base, base.len,
- object->type == OBJ_COMMIT);
+ object_type == OBJ_COMMIT);And finally, using the type we got from oid_object_info instead. Ok.
strbuf_release(&base);
free(data);
} else {
--
2.33.0.rc1.237.g0d66db33f3-googLGTM. Thanks. Reviewed-by: Emily Shaffer <redacted>