| Message ID | 921a79c8bd6ab688fb9f403a59eeaed3176b630e.1693262936.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e741c078721f8232da769a1100433d96c4393b32 |
| Headers | show |
| Series | pack-objects: support `--max-pack-size` for cruft packs | expand |
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);
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);
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. 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(-)