Re: [PATCH v2 6/8] commit.c: don't use deref_tag() -> object_as_type()
From: Jonathan Tan <hidden>
Date: 2021-04-21 22:26:46
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.
quoted hunk ↗ jump to hunk
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.