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 ;)
>
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> ---
> 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-goog
>
LGTM. Thanks.
Reviewed-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
