Re: [PATCH v2 6/8] commit.c: don't use deref_tag() -> object_as_type()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux