Re: [PATCH v2 15/20] for_each_object_in_pack(): convert to new revindex API

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> Avoid looking at the 'revindex' pointer directly and instead call
> 'pack_pos_to_index()'.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  packfile.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/packfile.c b/packfile.c
> index 936ab3def5..7bb1750934 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -2086,7 +2086,7 @@ int for_each_object_in_pack(struct packed_git *p,
>  		struct object_id oid;
>  
>  		if (flags & FOR_EACH_OBJECT_PACK_ORDER)
> -			pos = p->revindex[i].nr;
> +			pos = pack_pos_to_index(p, i);

It wasn't too bad before this series formally defined what
"position", "index" and "offset" mean, but now this has become
highly misleading. The variable "pos" here holds what we consider
"index" while "i" holds what we call "position" [*1*].

>  		else
>  			pos = i;

Perhaps renaming "uint32_t pos" to "nth" would avoid confusion?

-	if (nth_packed_object_id(&oid, p, pos) < 0)
+	if (nth_packed_object_id(&oid, p, nth) < 0)
		return error(...);


[Footnote]

*1* The nth_packed_object_id() call we make later using the value we
obtain here should be documented to take "index" as its last
parameter, now that is what we call the location in the index, which
is in object name order.