repack: move `pack_geometry` struct to the stack
The `pack_geometry` struct is used to maintain and partition a list of packfiles into a "frozen" set (to be left alone), and a non-frozen set (to be combined into a single new pack). In the previous commit, we removed a leak caused by neglecting to free() the heap allocated space used to store the structure itself. But there is no need for this structure to live on the heap anyway. Instead, let's move it to be stack allocated, eliminating the possibility of a direct leak like the one addressed in the previous patch. The one minor hitch is that we use the NULL-ness of the pack_geometry's struct pointer to determine whether or not we are performing a geometric repack with `--geometric=<d>`. But since we only initialize the pack_geometry structure when the `geometric_factor` is non-zero, we can use that variable (based on whether or not it is equal to zero) to determine whether or not we are performing a geometric repack. There are a couple of spots that have access to a pointer to the pack_geometry struct, but not the geometric_factor itself. Instead of passing in an additional variable, let's make the geometric_factor a field of the pack_geometry struct. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau committed
Aug 9, 2023 at 16:32 UTC
99d51978bed53e7453dfd0ec4f9c2edf5efff705
1 file changed
+31
-31
builtin/repack.c
+31
-31
@@ -303,6 +303,8 @@ struct pack_geometry {
303
struct packed_git **pack;
304
uint32_t pack_nr, pack_alloc;
305
uint32_t split;
306
+
307
+ int split_factor;
308
};
309
310
static uint32_t geometry_pack_weight(struct packed_git *p)
@@ -324,17 +326,13 @@ static int geometry_cmp(const void *va, const void *vb)
326
return 0;
327
}
328
327
-static void init_pack_geometry(struct pack_geometry **geometry_p,
329
+static void init_pack_geometry(struct pack_geometry *geometry,
330
struct string_list *existing_kept_packs,
331
const struct pack_objects_args *args)
332
{
333
struct packed_git *p;
332
- struct pack_geometry *geometry;
334
struct strbuf buf = STRBUF_INIT;
335
335
- *geometry_p = xcalloc(1, sizeof(struct pack_geometry));
336
- geometry = *geometry_p;
337
-
336
for (p = get_all_packs(the_repository); p; p = p->next) {
337
if (args->local && !p->pack_local)
338
/*
@@ -380,7 +378,7 @@ static void init_pack_geometry(struct pack_geometry **geometry_p,
378
strbuf_release(&buf);
379
}
380
383
-static void split_pack_geometry(struct pack_geometry *geometry, int factor)
381
+static void split_pack_geometry(struct pack_geometry *geometry)
382
{
383
uint32_t i;
384
uint32_t split;
@@ -399,12 +397,14 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor)
397
struct packed_git *ours = geometry->pack[i];
398
struct packed_git *prev = geometry->pack[i - 1];
399
402
- if (unsigned_mult_overflows(factor, geometry_pack_weight(prev)))
400
+ if (unsigned_mult_overflows(geometry->split_factor,
401
+ geometry_pack_weight(prev)))
402
die(_("pack %s too large to consider in geometric "
403
"progression"),
404
prev->pack_name);
405
407
- if (geometry_pack_weight(ours) < factor * geometry_pack_weight(prev))
406
+ if (geometry_pack_weight(ours) <
407
+ geometry->split_factor * geometry_pack_weight(prev))
408
break;
409
}
410
@@ -439,10 +439,12 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor)
439
for (i = split; i < geometry->pack_nr; i++) {
440
struct packed_git *ours = geometry->pack[i];
441
442
- if (unsigned_mult_overflows(factor, total_size))
442
+ if (unsigned_mult_overflows(geometry->split_factor,
443
+ total_size))
444
die(_("pack %s too large to roll up"), ours->pack_name);
445
445
- if (geometry_pack_weight(ours) < factor * total_size) {
446
+ if (geometry_pack_weight(ours) <
447
+ geometry->split_factor * total_size) {
448
if (unsigned_add_overflows(total_size,
449
geometry_pack_weight(ours)))
450
die(_("pack %s too large to roll up"),
@@ -498,7 +500,6 @@ static void free_pack_geometry(struct pack_geometry *geometry)
500
return;
501
502
free(geometry->pack);
501
- free(geometry);
503
}
504
505
struct midx_snapshot_ref_data {
@@ -575,7 +576,7 @@ static void midx_included_packs(struct string_list *include,
576
string_list_insert(include, xstrfmt("%s.idx", item->string));
577
for_each_string_list_item(item, names)
578
string_list_insert(include, xstrfmt("pack-%s.idx", item->string));
578
- if (geometry) {
579
+ if (geometry->split_factor) {
580
struct strbuf buf = STRBUF_INIT;
581
uint32_t i;
582
for (i = geometry->split; i < geometry->pack_nr; i++) {
@@ -779,7 +780,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
780
struct string_list names = STRING_LIST_INIT_DUP;
781
struct string_list existing_nonkept_packs = STRING_LIST_INIT_DUP;
782
struct string_list existing_kept_packs = STRING_LIST_INIT_DUP;
782
- struct pack_geometry *geometry = NULL;
783
+ struct pack_geometry geometry = { 0 };
784
struct strbuf line = STRBUF_INIT;
785
struct tempfile *refs_snapshot = NULL;
786
int i, ext, ret;
@@ -793,7 +794,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
794
struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
795
struct pack_objects_args po_args = {NULL};
796
struct pack_objects_args cruft_po_args = {NULL};
796
- int geometric_factor = 0;
797
int write_midx = 0;
798
const char *cruft_expiration = NULL;
799
const char *expire_to = NULL;
@@ -842,7 +842,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
842
N_("repack objects in packs marked with .keep")),
843
OPT_STRING_LIST(0, "keep-pack", &keep_pack_list, N_("name"),
844
N_("do not repack this pack")),
845
- OPT_INTEGER('g', "geometric", &geometric_factor,
845
+ OPT_INTEGER('g', "geometric", &geometry.split_factor,
846
N_("find a geometric progression with factor <N>")),
847
OPT_BOOL('m', "write-midx", &write_midx,
848
N_("write a multi-pack index of the resulting packs")),
@@ -918,11 +918,11 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
918
collect_pack_filenames(&existing_nonkept_packs, &existing_kept_packs,
919
&keep_pack_list);
920
921
- if (geometric_factor) {
921
+ if (geometry.split_factor) {
922
if (pack_everything)
923
die(_("options '%s' and '%s' cannot be used together"), "--geometric", "-A/-a");
924
init_pack_geometry(&geometry, &existing_kept_packs, &po_args);
925
- split_pack_geometry(geometry, geometric_factor);
925
+ split_pack_geometry(&geometry);
926
}
927
928
prepare_pack_objects(&cmd, &po_args, packtmp);
@@ -936,7 +936,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
936
strvec_pushf(&cmd.args, "--keep-pack=%s",
937
keep_pack_list.items[i].string);
938
strvec_push(&cmd.args, "--non-empty");
939
- if (!geometry) {
939
+ if (!geometry.split_factor) {
940
/*
941
* We need to grab all reachable objects, including those that
942
* are reachable from reflogs and the index.
@@ -983,7 +983,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
983
strvec_push(&cmd.args, "--pack-loose-unreachable");
984
}
985
}
986
- } else if (geometry) {
986
+ } else if (geometry.split_factor) {
987
strvec_push(&cmd.args, "--stdin-packs");
988
strvec_push(&cmd.args, "--unpacked");
989
} else {
@@ -991,7 +991,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
991
strvec_push(&cmd.args, "--incremental");
992
}
993
994
- if (geometry)
994
+ if (geometry.split_factor)
995
cmd.in = -1;
996
else
997
cmd.no_stdin = 1;
@@ -1000,17 +1000,17 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1000
if (ret)
1001
goto cleanup;
1002
1003
- if (geometry) {
1003
+ if (geometry.split_factor) {
1004
FILE *in = xfdopen(cmd.in, "w");
1005
/*
1006
* The resulting pack should contain all objects in packs that
1007
* are going to be rolled up, but exclude objects in packs which
1008
* are being left alone.
1009
*/
1010
- for (i = 0; i < geometry->split; i++)
1011
- fprintf(in, "%s\n", pack_basename(geometry->pack[i]));
1012
- for (i = geometry->split; i < geometry->pack_nr; i++)
1013
- fprintf(in, "^%s\n", pack_basename(geometry->pack[i]));
1010
+ for (i = 0; i < geometry.split; i++)
1011
+ fprintf(in, "%s\n", pack_basename(geometry.pack[i]));
1012
+ for (i = geometry.split; i < geometry.pack_nr; i++)
1013
+ fprintf(in, "^%s\n", pack_basename(geometry.pack[i]));
1014
fclose(in);
1015
}
1016
@@ -1153,9 +1153,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1153
if (write_midx) {
1154
struct string_list include = STRING_LIST_INIT_NODUP;
1155
midx_included_packs(&include, &existing_nonkept_packs,
1156
- &existing_kept_packs, &names, geometry);
1156
+ &existing_kept_packs, &names, &geometry);
1157
1158
- ret = write_midx_included_packs(&include, geometry,
1158
+ ret = write_midx_included_packs(&include, &geometry,
1159
refs_snapshot ? get_tempfile_path(refs_snapshot) : NULL,
1160
show_progress, write_bitmaps > 0);
1161
@@ -1178,12 +1178,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1178
remove_redundant_pack(packdir, item->string);
1179
}
1180
1181
- if (geometry) {
1181
+ if (geometry.split_factor) {
1182
struct strbuf buf = STRBUF_INIT;
1183
1184
uint32_t i;
1185
- for (i = 0; i < geometry->split; i++) {
1186
- struct packed_git *p = geometry->pack[i];
1185
+ for (i = 0; i < geometry.split; i++) {
1186
+ struct packed_git *p = geometry.pack[i];
1187
if (string_list_has_string(&names,
1188
hash_to_hex(p->hash)))
1189
continue;
@@ -1226,7 +1226,7 @@ cleanup:
1226
string_list_clear(&names, 1);
1227
string_list_clear(&existing_nonkept_packs, 0);
1228
string_list_clear(&existing_kept_packs, 0);
1229
- free_pack_geometry(geometry);
1229
+ free_pack_geometry(&geometry);
1230
1231
return ret;
1232
}