Re: [PATCH 05/20] check_object(): convert to new revindex API
From: Derrick Stolee <hidden>
Date: 2021-01-11 11:44:23
On 1/8/2021 1:17 PM, Taylor Blau wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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))
quoted hunk ↗ jump to hunk
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