Subject: Re: [PATCH 1/4] builtin/pack-objects.c: remove unnecessary strbuf_reset()

From: Junio C Hamano <[email-masked]>
Date: Tue, 29 Aug 2023 10:34:43 -0700

Taylor Blau <[email-masked]> 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 <[email-masked]>
> ---
>  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);