| Message ID | 1bfd2fb6ab01af689fc6495e2f32d3f64c19f4b7.1650547400.git.dyroneteng@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5dcee7c7059307ca3a9d5c1a5c6551deb25cc3dc |
| Headers | show |
| Series | trace2 output for bitmap decision path | expand |
On Thu, Apr 21, 2022 at 09:26:36PM +0800, Teng Long wrote: > In "open_midx_bitmap()", we do a loop with the MIDX(es) in repo, when > the first one has been found, then will break out by a "return" > directly. > > But actually, it's better to don't stop the loop until we have visited s/don't stop/continue > both the MIDX in our repository, as well as any alternates (along with > _their_ alternates, recursively). > > The discussion link of community: > > https://public-inbox.org/git/YjzCTLLDCby+kJrZ@nand.local/ In the future, it is often worth summarizing the discussion, optionally linking off to the list archive. In this case, I wouldn't mind a little more detail beyond "it's better to [continue] the loop until ...". In particular, you say it's better without saying why that is the case. The link to <YjzCTLLDCby+kJrZ@nand.local> explains, but this commit message as-is leaves out some details that are important IMHO. > --- > pack-bitmap.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) The actual changes look good to me. Thanks, Taylor
diff --git a/pack-bitmap.c b/pack-bitmap.c index 9c666cdb8b..112c2b12c6 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -494,15 +494,16 @@ static int open_pack_bitmap(struct repository *r, static int open_midx_bitmap(struct repository *r, struct bitmap_index *bitmap_git) { + int ret = -1; struct multi_pack_index *midx; assert(!bitmap_git->map); for (midx = get_multi_pack_index(r); midx; midx = midx->next) { if (!open_midx_bitmap_1(bitmap_git, midx)) - return 0; + ret = 0; } - return -1; + return ret; } static int open_bitmap(struct repository *r,
In "open_midx_bitmap()", we do a loop with the MIDX(es) in repo, when the first one has been found, then will break out by a "return" directly. But actually, it's better to don't stop the loop until we have visited both the MIDX in our repository, as well as any alternates (along with _their_ alternates, recursively). The discussion link of community: https://public-inbox.org/git/YjzCTLLDCby+kJrZ@nand.local/ Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Teng Long <dyroneteng@gmail.com> --- pack-bitmap.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)