git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH 1/4] builtin/pack-objects.c: remove unnecessary strbuf_reset()
Date: Tue, 29 Aug 2023 10:34:43 -0700	[thread overview]
Message-ID: <xmqq4jkheor0.fsf@gitster.g> (raw)
In-Reply-To: <921a79c8bd6ab688fb9f403a59eeaed3176b630e.1693262936.git.me@ttaylorr.com> (Taylor Blau's message of "Mon, 28 Aug 2023 18:49:04 -0400")

Taylor Blau <me@ttaylorr.com> writes:

> When reading input with the `--cruft` option, `git pack-objects` reads
> each line into a strbuf, and then moves it to either the list of
> discarded or fresh packs, depending on whether or not the input line
> starts with a '-' character.
>
> At the beginning of each loop iteration, the next line of input is read
> with `strbuf_getline()`, which calls `strbuf_reset()` (as a part of
> `strbuf_getwholeline()`) before reading the next line of input.

Yup, the reset() at the end of each iteration is not needed to
prepare for the next interation (which getline() is perfectly
capable of doing it itself) but helps to release the resource
that is no longer used after the loop finishes iterating.  If I were
cleaning up this code, I would probably move the _reset() after the
loop instead *and* keep the _release() at the end, so that future
changes can add new (re)uses of buf in between.

But it is probalby not worth worrying about keeping a single line
from the cruft object list a little longer than absolutely needed.

Will queue.  Thanks.

>
> Thus, the call to `strbuf_reset()` (added back in b757353676
> (builtin/pack-objects.c: --cruft without expiration, 2022-05-20)) at the
> end of the loop is unnecessary, so let's remove it accordingly.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  builtin/pack-objects.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index d2a162d528..868efe7e0f 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -3603,7 +3603,6 @@ static void read_cruft_objects(void)
>  			string_list_append(&discard_packs, buf.buf + 1);
>  		else
>  			string_list_append(&fresh_packs, buf.buf);
> -		strbuf_reset(&buf);
>  	}
>  
>  	string_list_sort(&discard_packs);

  reply	other threads:[~2023-08-29 17:35 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-28 22:48 [PATCH 0/4] pack-objects: support `--max-pack-size` for cruft packs Taylor Blau
2023-08-28 22:49 ` [PATCH 1/4] builtin/pack-objects.c: remove unnecessary strbuf_reset() Taylor Blau
2023-08-29 17:34   ` Junio C Hamano [this message]
2023-08-28 22:49 ` [PATCH 2/4] builtin/pack-objects.c: support `--max-pack-size` with `--cruft` Taylor Blau
2023-08-29 17:42   ` Junio C Hamano
2023-08-29 17:52     ` Taylor Blau
2023-08-28 22:49 ` [PATCH 3/4] Documentation/gitformat-pack.txt: remove multi-cruft packs alternative Taylor Blau
2023-08-28 22:49 ` [PATCH 4/4] Documentation/gitformat-pack.txt: drop mixed version section Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq4jkheor0.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).