| Message ID | patch-6.8-f337a5442d-20210420T133218Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | object.c: add and use "is expected" utility function + object_as_type() use | expand |
> Change a use of the object_as_type() function introduced in > 8ff226a9d5e (add object_as_type helper for casting objects, > 2014-07-13) to instead assume that we're not dealing with OBJ_NONE (or > OBJ_BAD) from deref_tag(). > > This makes this code easier to read, as the reader isn't wondering why > the function would need to deal with that. We're simply doing a check > of OBJ_{COMMIT,TREE,BLOB,TAG} here, not the bare-bones initialization > object_as_type() might be called on to do. I think the benefit of using object_as_type() here (functionality that checks the object type, with optional "quiet" behavior) outweighs the drawback (additional functionality that we don't need). If we're worried that the reader would wonder about the OBJ_NONE case, we can include the BUG check as you did. > Even though we can read deref_tag() and see that it won't return > OBJ_NONE and friends, let's add a BUG() assertion here to help future > maintenance. This is reasonable. > diff --git a/commit.c b/commit.c > index 3d7f1fba0c..c3bc6cbec4 100644 > --- a/commit.c > +++ b/commit.c > @@ -31,13 +31,22 @@ const char *commit_type = "commit"; > struct commit *lookup_commit_reference_gently(struct repository *r, > const struct object_id *oid, int quiet) > { > - struct object *obj = deref_tag(r, > - parse_object(r, oid), > - NULL, 0); > + struct object *tmp = parse_object(r, oid); > + struct object *obj = deref_tag(r, tmp, NULL, 0); This change isn't unnecessary, I think. "tmp" isn't used anywhere else.
diff --git a/commit.c b/commit.c index 3d7f1fba0c..c3bc6cbec4 100644 --- a/commit.c +++ b/commit.c @@ -31,13 +31,22 @@ const char *commit_type = "commit"; struct commit *lookup_commit_reference_gently(struct repository *r, const struct object_id *oid, int quiet) { - struct object *obj = deref_tag(r, - parse_object(r, oid), - NULL, 0); + struct object *tmp = parse_object(r, oid); + struct object *obj = deref_tag(r, tmp, NULL, 0); if (!obj) return NULL; - return object_as_type(obj, OBJ_COMMIT, quiet); + + if (obj->type <= 0) + BUG("should have initialized obj->type = OBJ_{COMMIT,TREE,BLOB,TAG} from deref_tag()"); + if (obj->type != OBJ_COMMIT) { + if (!quiet) { + enum object_type have = obj->type; + oid_is_type_or_error(oid, OBJ_COMMIT, &have); + } + return NULL; + } + return (struct commit *)obj; } struct commit *lookup_commit_reference(struct repository *r, const struct object_id *oid)
Change a use of the object_as_type() function introduced in 8ff226a9d5e (add object_as_type helper for casting objects, 2014-07-13) to instead assume that we're not dealing with OBJ_NONE (or OBJ_BAD) from deref_tag(). This makes this code easier to read, as the reader isn't wondering why the function would need to deal with that. We're simply doing a check of OBJ_{COMMIT,TREE,BLOB,TAG} here, not the bare-bones initialization object_as_type() might be called on to do. Even though we can read deref_tag() and see that it won't return OBJ_NONE and friends, let's add a BUG() assertion here to help future maintenance. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- commit.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)