[PATCH 3/4] traverse_bitmap_commit_list(): don't free result

Since it was introduced in fff42755ef (pack-bitmap: add support for bitmap indexes, 2013-12-21), this function has freed the result after traversing it. That is an artifact of the early days of the bitmap code, when we had a single static "struct bitmap_index". Back then, it was intended that you would do:

  prepare_bitmap_walk(&revs);
  traverse_bitmap_commit_list(&revs);

Since the actual bitmap_index struct was totally behind the scenes, it was convenient for traverse_bitmap_commit_list() to clean it up, clearing the way for another traversal.

But since 3ae5fa0768 (pack-bitmap: remove bitmap_git global variable, 2018-06-07), the caller explicitly manages the bitmap_index struct itself, like this:

  b = prepare_bitmap_walk(&revs);
  traverse_bitmap_commit_list(b, &revs);
  free_bitmap_index(b);

It no longer makes sense to auto-free the result after the traversal. If you want to do another traversal, you'd just create a new bitmap_index. And while nobody tries to call traverse_bitmap_commit_list() twice, the fact that it throws away the result might be surprising, and is better avoided.

Note that in the "old" way it was possible for two walks to amortize the cost of opening the on-disk .bitmap file (since it was stored in the global bitmap_index), but we lost that in 3ae5fa0768. However, no code actually does this, so it's not worth addressing now. The solution might involve a new:

  reset_bitmap_walk(b, &revs);

call. Or we might even attach the bitmap data to its matching packed_git struct, so that multiple prepare_bitmap_walk() calls could use it. That can wait until somebody actually has need of the optimization (and until then, we'll do the correct, unsurprising thing).

Signed-off-by: Jeff King <peff@peff.net>
---
 pack-bitmap.c | 3 ---
 1 file changed, 3 deletions(-)