| Message ID | 30ead880b38c5f572e609554b075ef81ff80ad87.1628618950.git.jonathantanmy@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | In grep, no adding submodule ODB as alternates | expand |
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@google.com> > --- > 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@google.com>
> > - 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? For the purposes of grep, I don't think that this matters - we just want to find the ultimate tree that this object represents.
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); obj_read_unlock(); data = read_object_with_reference(&subrepo, - &object->oid, tree_type, + oid, tree_type, &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); strbuf_release(&base); free(data); } else {
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(). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- builtin/grep.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)