diff mbox series

[v5,3/4] pack-bitmap.c: break out of the bitmap loop early if not tracing

Message ID 22deec6aab6649b228af7d858b93672f9ce2b925.1669644101.git.dyroneteng@gmail.com (mailing list archive)
State Accepted
Commit 833f4c0514fb08368066ace5260b4b80792c4ffc
Headers show
Series pack-bitmap.c: avoid exposing absolute paths | expand

Commit Message

Teng Long Nov. 28, 2022, 2:09 p.m. UTC 
From: Jeff King <peff@peff.net>

After opening a bitmap successfully, we try opening others only
because we want to report that other bitmap files are ignored in
the trace2 log.  When trace2 is not enabled, we do not have to
do any of that.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Teng Long <dyroneteng@gmail.com>
---
 pack-bitmap.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Taylor Blau Nov. 28, 2022, 11:26 p.m. UTC | #1 
On Mon, Nov 28, 2022 at 10:09:52PM +0800, Teng Long wrote:
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index aaa2d9a104..3b6c2f804a 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -527,8 +527,15 @@ static int open_pack_bitmap(struct repository *r,
>  	assert(!bitmap_git->map);
>
>  	for (p = get_all_packs(r); p; p = p->next) {
> -		if (open_pack_bitmap_1(bitmap_git, p) == 0)
> +		if (open_pack_bitmap_1(bitmap_git, p) == 0) {

While we're here, we *could* change this line to read:

    if (!open_pack_bitmap_1(bitmap_git, p))

which more closely adheres to our conventions. But I don't think just
because we could do something necessarily means that we have to, so I'm
happy to leave it alone, too. It definitely does not merit a reroll on
its own.

Otherwise this patch looks quite reasonable.

Thanks,
Taylor
Teng Long Nov. 29, 2022, 1:17 p.m. UTC | #2 
Taylor Blau <me@ttaylorr.com> writes:

> > diff --git a/pack-bitmap.c b/pack-bitmap.c
> > index aaa2d9a104..3b6c2f804a 100644
> > --- a/pack-bitmap.c
> > +++ b/pack-bitmap.c
> > @@ -527,8 +527,15 @@ static int open_pack_bitmap(struct repository *r,
> >  	assert(!bitmap_git->map);
> >
> >  	for (p = get_all_packs(r); p; p = p->next) {
> > -		if (open_pack_bitmap_1(bitmap_git, p) == 0)
> > +		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
>
> While we're here, we *could* change this line to read:
>
>     if (!open_pack_bitmap_1(bitmap_git, p))

Feeling a bit sad why I didn't notice here.

> which more closely adheres to our conventions. But I don't think just
> because we could do something necessarily means that we have to, so I'm
> happy to leave it alone, too. It definitely does not merit a reroll on
> its own.
>
> Otherwise this patch looks quite reasonable.

That's a good catch and thanks for your suggestion.
diff mbox series

Patch

diff --git a/pack-bitmap.c b/pack-bitmap.c
index aaa2d9a104..3b6c2f804a 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -527,8 +527,15 @@  static int open_pack_bitmap(struct repository *r,
 	assert(!bitmap_git->map);
 
 	for (p = get_all_packs(r); p; p = p->next) {
-		if (open_pack_bitmap_1(bitmap_git, p) == 0)
+		if (open_pack_bitmap_1(bitmap_git, p) == 0) {
 			ret = 0;
+			/*
+			 * The only reason to keep looking is to report
+			 * duplicates.
+			 */
+			if (!trace2_is_enabled())
+				break;
+		}
 	}
 
 	return ret;