Re: What's cooking in git.git (Sep 2018, #04; Thu, 20)
- From
René Scharfe <l.s.r@web.de>
- Date
- Oct 1, 2018, 19:16 UTC
- Message-ID
- <c5f7009b-53b5-26b9-8a01-b8648ee33a15@web.de>
- In-Reply-To
- <xmqqin2zxw55.fsf@gitster-ct.c.googlers.com>
Am 21.09.2018 um 07:22 schrieb Junio C Hamano:
Show 13 quoted lines
> * cc/delta-islands (2018-08-16) 7 commits
> (merged to 'next' on 2018-08-27 at cf3d7bd93f)
> + pack-objects: move 'layer' into 'struct packing_data'
> + pack-objects: move tree_depth into 'struct packing_data'
> + t5320: tests for delta islands
> + repack: add delta-islands support
> + pack-objects: add delta-islands support
> + pack-objects: refactor code into compute_layer_order()
> + Add delta-islands.{c,h}
>
> Lift code from GitHub to restrict delta computation so that an
> object that exists in one fork is not made into a delta against
> another object that does not appear in the same forked repository.Show 15 quoted lines
> * jk/pack-delta-reuse-with-bitmap (2018-08-21) 6 commits > (merged to 'next' on 2018-08-22 at fc50b59dab) > + pack-objects: reuse on-disk deltas for thin "have" objects > + pack-bitmap: save "have" bitmap from walk > + t/perf: add perf tests for fetches from a bitmapped server > + t/perf: add infrastructure for measuring sizes > + t/perf: factor out percent calculations > + t/perf: factor boilerplate out of test_perf > (this branch is used by jk/pack-objects-with-bitmap-fix.) > > When creating a thin pack, which allows objects to be made into a > delta against another object that is not in the resulting pack but > is known to be present on the receiving end, the code learned to > take advantage of the reachability bitmap; this allows the server > to send a delta against a base beyond the "boundary" commit.
Not sure if it's the interaction of the two topics or if only one of them is to blame, but the result of the merge can dereference a NULL pointer. Found using Clang's UBSan and t5310.
Here's a patch that avoids the issue, but I don't know if it's the right thing to do -- should we rather treat a non-existing base as "not from the same island" instead?
And it's certainly ugly -- that condition is complicated enough already. Splitting it up in a nice way would probably help, but how?
--- builtin/pack-objects.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index e6316d294d..9abed4a0f0 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1559,7 +1559,8 @@ static void check_object(struct object_entry *entry) (base_entry = packlist_find(&to_pack, base_ref, NULL)) || (thin && bitmap_has_sha1_in_uninteresting(bitmap_git, base_ref))) && - in_same_island(&entry->idx.oid, &base_entry->idx.oid)) { + (!base_entry || + in_same_island(&entry->idx.oid, &base_entry->idx.oid))) { /* * If base_ref was set above that means we wish to * reuse delta data, and either we found that object in
-- 2.19.0