@matheusbsbs / matheus-bagliano / commits / 799237852b

reftable: pass opts as constant pointer

We sometimes pass the refatble write options as value and sometimes as a pointer. This is quite confusing and makes the reader wonder whether the options get modified sometimes. In fact, `reftable_new_writer()` does cause the caller-provided options to get updated when some values aren't set up. This is quite unexpected, but didn't cause any harm until now. Adapt the code so that we do not modify the caller-provided values anymore. While at it, refactor the code to code to consistently pass the options as a constant pointer to clarify that the caller-provided opts will not ever get modified. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed May 13, 2024 at 10:17 UTC 799237852bd265feb1e1a8d4a19780e20991b4fd

7 files changed +46 -38

refs/reftable-backend.c

+3 -3

@@ -129,7 +129,7 @@ static struct reftable_stack *stack_for(struct reftable_ref_store *store,

129 store->base.repo->commondir, wtname_buf.buf);

130

131 store->err = reftable_new_stack(&stack, wt_dir.buf,

132 - store->write_options);

132 + &store->write_options);

133 assert(store->err != REFTABLE_API_ERROR);

134 strmap_put(&store->worktree_stacks, wtname_buf.buf, stack);

135 }

@@ -263,7 +263,7 @@ static struct ref_store *reftable_be_init(struct repository *repo,

263 }

264 strbuf_addstr(&path, "/reftable");

265 refs->err = reftable_new_stack(&refs->main_stack, path.buf,

266 - refs->write_options);

266 + &refs->write_options);

267 if (refs->err)

268 goto done;

269

@@ -280,7 +280,7 @@ static struct ref_store *reftable_be_init(struct repository *repo,

280 strbuf_addf(&path, "%s/reftable", gitdir);

281

282 refs->err = reftable_new_stack(&refs->worktree_stack, path.buf,

283 - refs->write_options);

283 + &refs->write_options);

284 if (refs->err)

285 goto done;

286 }

reftable/dump.c

+1 -1

@@ -29,7 +29,7 @@ static int compact_stack(const char *stackdir)

29 struct reftable_stack *stack = NULL;

30 struct reftable_write_options opts = { 0 };

32 - int err = reftable_new_stack(&stack, stackdir, opts);

32 + int err = reftable_new_stack(&stack, stackdir, &opts);

33 if (err < 0)

34 goto done;

reftable/reftable-stack.h

+1 -1

@@ -29,7 +29,7 @@ struct reftable_stack;

29 * stored in 'dir'. Typically, this should be .git/reftables.

30 */

31 int reftable_new_stack(struct reftable_stack **dest, const char *dir,

32 - struct reftable_write_options opts);

32 + const struct reftable_write_options *opts);

34 /* returns the update_index at which a next table should be written. */

35 uint64_t reftable_stack_next_update_index(struct reftable_stack *st);

reftable/reftable-writer.h

+1 -1

@@ -88,7 +88,7 @@ struct reftable_stats {

88 struct reftable_writer *

89 reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),

90 int (*flush_func)(void *),

91 - void *writer_arg, struct reftable_write_options *opts);

91 + void *writer_arg, const struct reftable_write_options *opts);

93 /* Set the range of update indices for the records we will add. When writing a

94 table into a stack, the min should be at least

reftable/stack.c

+5 -2

@@ -54,12 +54,15 @@ static int reftable_fd_flush(void *arg)

54 }

56 int reftable_new_stack(struct reftable_stack **dest, const char *dir,

57 - struct reftable_write_options opts)

57 + const struct reftable_write_options *_opts)

58 {

59 struct reftable_stack *p = reftable_calloc(1, sizeof(*p));

60 struct strbuf list_file_name = STRBUF_INIT;

61 + struct reftable_write_options opts = {0};

62 int err = 0;

64 + if (_opts)

65 + opts = *_opts;

66 if (opts.hash_id == 0)

67 opts.hash_id = GIT_SHA1_FORMAT_ID;

@@ -1438,7 +1441,7 @@ int reftable_stack_print_directory(const char *stackdir, uint32_t hash_id)

1441 struct reftable_merged_table *merged = NULL;

1442 struct reftable_table table = { NULL };

1443

1441 - int err = reftable_new_stack(&stack, stackdir, opts);

1444 + int err = reftable_new_stack(&stack, stackdir, &opts);

1445 if (err < 0)

1446 goto done;

1447

reftable/stack_test.c

+24 -24

@@ -163,7 +163,7 @@ static void test_reftable_stack_add_one(void)

163 };

164 struct reftable_ref_record dest = { NULL };

165 struct stat stat_result = { 0 };

166 - err = reftable_new_stack(&st, dir, opts);

166 + err = reftable_new_stack(&st, dir, &opts);

167 EXPECT_ERR(err);

168

169 err = reftable_stack_add(st, &write_test_ref, &ref);

@@ -232,10 +232,10 @@ static void test_reftable_stack_uptodate(void)

232 /* simulate multi-process access to the same stack

233 by creating two stacks for the same directory.

234 */

235 - err = reftable_new_stack(&st1, dir, opts);

235 + err = reftable_new_stack(&st1, dir, &opts);

236 EXPECT_ERR(err);

237

238 - err = reftable_new_stack(&st2, dir, opts);

238 + err = reftable_new_stack(&st2, dir, &opts);

239 EXPECT_ERR(err);

240

241 err = reftable_stack_add(st1, &write_test_ref, &ref1);

@@ -270,7 +270,7 @@ static void test_reftable_stack_transaction_api(void)

270 };

271 struct reftable_ref_record dest = { NULL };

272

273 - err = reftable_new_stack(&st, dir, opts);

273 + err = reftable_new_stack(&st, dir, &opts);

274 EXPECT_ERR(err);

275

276 reftable_addition_destroy(add);

@@ -304,7 +304,7 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void)

304 struct reftable_stack *st = NULL;

305 int i, n = 20, err;

306

307 - err = reftable_new_stack(&st, dir, opts);

307 + err = reftable_new_stack(&st, dir, &opts);

308 EXPECT_ERR(err);

309

310 for (i = 0; i <= n; i++) {

@@ -365,7 +365,7 @@ static void test_reftable_stack_auto_compaction_fails_gracefully(void)

365 char *dir = get_tmp_dir(__LINE__);

366 int err;

367

368 - err = reftable_new_stack(&st, dir, opts);

368 + err = reftable_new_stack(&st, dir, &opts);

369 EXPECT_ERR(err);

370

371 err = reftable_stack_add(st, write_test_ref, &ref);

@@ -418,7 +418,7 @@ static void test_reftable_stack_update_index_check(void)

418 .value.symref = "master",

419 };

420

421 - err = reftable_new_stack(&st, dir, opts);

421 + err = reftable_new_stack(&st, dir, &opts);

422 EXPECT_ERR(err);

423

424 err = reftable_stack_add(st, &write_test_ref, &ref1);

@@ -437,7 +437,7 @@ static void test_reftable_stack_lock_failure(void)

437 struct reftable_stack *st = NULL;

438 int err, i;

439

440 - err = reftable_new_stack(&st, dir, opts);

440 + err = reftable_new_stack(&st, dir, &opts);

441 EXPECT_ERR(err);

442 for (i = -1; i != REFTABLE_EMPTY_TABLE_ERROR; i--) {

443 err = reftable_stack_add(st, &write_error, &i);

@@ -465,7 +465,7 @@ static void test_reftable_stack_add(void)

465 struct stat stat_result;

466 int N = ARRAY_SIZE(refs);

467

468 - err = reftable_new_stack(&st, dir, opts);

468 + err = reftable_new_stack(&st, dir, &opts);

469 EXPECT_ERR(err);

470

471 for (i = 0; i < N; i++) {

@@ -575,7 +575,7 @@ static void test_reftable_stack_log_normalize(void)

575 .update_index = 1,

576 };

577

578 - err = reftable_new_stack(&st, dir, opts);

578 + err = reftable_new_stack(&st, dir, &opts);

579 EXPECT_ERR(err);

580

581 input.value.update.message = "one\ntwo";

@@ -617,7 +617,7 @@ static void test_reftable_stack_tombstone(void)

617 struct reftable_ref_record dest = { NULL };

618 struct reftable_log_record log_dest = { NULL };

619

620 - err = reftable_new_stack(&st, dir, opts);

620 + err = reftable_new_stack(&st, dir, &opts);

621 EXPECT_ERR(err);

622

623 /* even entries add the refs, odd entries delete them. */

@@ -701,18 +701,18 @@ static void test_reftable_stack_hash_id(void)

701 struct reftable_stack *st_default = NULL;

702 struct reftable_ref_record dest = { NULL };

703

704 - err = reftable_new_stack(&st, dir, opts);

704 + err = reftable_new_stack(&st, dir, &opts);

705 EXPECT_ERR(err);

706

707 err = reftable_stack_add(st, &write_test_ref, &ref);

708 EXPECT_ERR(err);

709

710 /* can't read it with the wrong hash ID. */

711 - err = reftable_new_stack(&st32, dir, opts32);

711 + err = reftable_new_stack(&st32, dir, &opts32);

712 EXPECT(err == REFTABLE_FORMAT_ERROR);

713

714 /* check that we can read it back with default opts too. */

715 - err = reftable_new_stack(&st_default, dir, opts_default);

715 + err = reftable_new_stack(&st_default, dir, &opts_default);

716 EXPECT_ERR(err);

717

718 err = reftable_stack_read_ref(st_default, "master", &dest);

@@ -756,7 +756,7 @@ static void test_reflog_expire(void)

756 };

757 struct reftable_log_record log = { NULL };

758

759 - err = reftable_new_stack(&st, dir, opts);

759 + err = reftable_new_stack(&st, dir, &opts);

760 EXPECT_ERR(err);

761

762 for (i = 1; i <= N; i++) {

@@ -825,13 +825,13 @@ static void test_empty_add(void)

825 char *dir = get_tmp_dir(__LINE__);

826 struct reftable_stack *st2 = NULL;

827

828 - err = reftable_new_stack(&st, dir, opts);

828 + err = reftable_new_stack(&st, dir, &opts);

829 EXPECT_ERR(err);

830

831 err = reftable_stack_add(st, &write_nothing, NULL);

832 EXPECT_ERR(err);

833

834 - err = reftable_new_stack(&st2, dir, opts);

834 + err = reftable_new_stack(&st2, dir, &opts);

835 EXPECT_ERR(err);

836 clear_dir(dir);

837 reftable_stack_destroy(st);

@@ -858,7 +858,7 @@ static void test_reftable_stack_auto_compaction(void)

858 int err, i;

859 int N = 100;

860

861 - err = reftable_new_stack(&st, dir, opts);

861 + err = reftable_new_stack(&st, dir, &opts);

862 EXPECT_ERR(err);

863

864 for (i = 0; i < N; i++) {

@@ -894,7 +894,7 @@ static void test_reftable_stack_add_performs_auto_compaction(void)

894 char *dir = get_tmp_dir(__LINE__);

895 int err, i, n = 20;

896

897 - err = reftable_new_stack(&st, dir, opts);

897 + err = reftable_new_stack(&st, dir, &opts);

898 EXPECT_ERR(err);

899

900 for (i = 0; i <= n; i++) {

@@ -942,7 +942,7 @@ static void test_reftable_stack_compaction_concurrent(void)

942 int err, i;

943 int N = 3;

944

945 - err = reftable_new_stack(&st1, dir, opts);

945 + err = reftable_new_stack(&st1, dir, &opts);

946 EXPECT_ERR(err);

947

948 for (i = 0; i < N; i++) {

@@ -959,7 +959,7 @@ static void test_reftable_stack_compaction_concurrent(void)

959 EXPECT_ERR(err);

960 }

961

962 - err = reftable_new_stack(&st2, dir, opts);

962 + err = reftable_new_stack(&st2, dir, &opts);

963 EXPECT_ERR(err);

964

965 err = reftable_stack_compact_all(st1, NULL);

@@ -991,7 +991,7 @@ static void test_reftable_stack_compaction_concurrent_clean(void)

991 int err, i;

992 int N = 3;

993

994 - err = reftable_new_stack(&st1, dir, opts);

994 + err = reftable_new_stack(&st1, dir, &opts);

995 EXPECT_ERR(err);

996

997 for (i = 0; i < N; i++) {

@@ -1008,7 +1008,7 @@ static void test_reftable_stack_compaction_concurrent_clean(void)

1008 EXPECT_ERR(err);

1009 }

1010

1011 - err = reftable_new_stack(&st2, dir, opts);

1011 + err = reftable_new_stack(&st2, dir, &opts);

1012 EXPECT_ERR(err);

1013

1014 err = reftable_stack_compact_all(st1, NULL);

@@ -1017,7 +1017,7 @@ static void test_reftable_stack_compaction_concurrent_clean(void)

1017 unclean_stack_close(st1);

1018 unclean_stack_close(st2);

1019

1020 - err = reftable_new_stack(&st3, dir, opts);

1020 + err = reftable_new_stack(&st3, dir, &opts);

1021 EXPECT_ERR(err);

1022

1023 err = reftable_stack_clean(st3);

reftable/writer.c

+11 -6

@@ -122,20 +122,25 @@ static struct strbuf reftable_empty_strbuf = STRBUF_INIT;

122 struct reftable_writer *

123 reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),

124 int (*flush_func)(void *),

125 - void *writer_arg, struct reftable_write_options *opts)

125 + void *writer_arg, const struct reftable_write_options *_opts)

126 {

127 struct reftable_writer *wp = reftable_calloc(1, sizeof(*wp));

128 - strbuf_init(&wp->block_writer_data.last_key, 0);

129 - options_set_defaults(opts);

130 - if (opts->block_size >= (1 << 24)) {

128 + struct reftable_write_options opts = {0};

129 +

130 + if (_opts)

131 + opts = *_opts;

132 + options_set_defaults(&opts);

133 + if (opts.block_size >= (1 << 24)) {

134 /* TODO - error return? */

135 abort();

136 }

137 +

138 + strbuf_init(&wp->block_writer_data.last_key, 0);

139 wp->last_key = reftable_empty_strbuf;

135 - REFTABLE_CALLOC_ARRAY(wp->block, opts->block_size);

140 + REFTABLE_CALLOC_ARRAY(wp->block, opts.block_size);

141 wp->write = writer_func;

142 wp->write_arg = writer_arg;

138 - wp->opts = *opts;

143 + wp->opts = opts;

144 wp->flush = flush_func;

145 writer_reinit_block_writer(wp, BLOCK_TYPE_REF);

146