On 1/8/2021 1:17 PM, Taylor Blau wrote:
> Replace direct accesses to the revindex with calls to
> 'offset_to_pack_pos()' and 'pack_pos_to_index()'.
>
> Since this caller already had some error checking (it can jump to the
> 'give_up' label if it encounters an error), we can easily check whether
> or not the provided offset points to an object in the given pack. This
> error checking existed prior to this patch, too, since the caller checks
> whether the return value from 'find_pack_revindex()' was NULL or not.
>
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> builtin/pack-objects.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 4341bc27b4..a193cdaf2f 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1813,11 +1813,11 @@ static void check_object(struct object_entry *entry, uint32_t object_index)
> goto give_up;
> }
> if (reuse_delta && !entry->preferred_base) {
> - struct revindex_entry *revidx;
> - revidx = find_pack_revindex(p, ofs);
> - if (!revidx)
> + uint32_t pos;
> + if (offset_to_pack_pos(p, ofs, &pos) < 0)
The current implementation does not return a positive value. Only
-1 on error and 0 on success. Is this "< 0" doing anything important?
Seems like it would be easiest to do
if (offset_to_pack_pos(p, ofs, &pos))
> goto give_up;
> - if (!nth_packed_object_id(&base_ref, p, revidx->nr))
> + if (!nth_packed_object_id(&base_ref, p,
> + pack_pos_to_index(p, pos)))
> have_base = 1;
> }
Thanks,
-Stolee
