@matheusbsbs / matheus-bagliano / commits / cec2b6f55a

refs/iterator: separate lifecycle from iteration

The ref and reflog iterators have their lifecycle attached to iteration: once the iterator reaches its end, it is automatically released and the caller doesn't have to care about that anymore. When the iterator should be released before it has been exhausted, callers must explicitly abort the iterator via `ref_iterator_abort()`. This lifecycle is somewhat unusual in the Git codebase and creates two problems: - Callsites need to be very careful about when exactly they call `ref_iterator_abort()`, as calling the function is only valid when the iterator itself still is. This leads to somewhat awkward calling patterns in some situations. - It is impossible to reuse iterators and re-seek them to a different prefix. This feature isn't supported by any iterator implementation except for the reftable iterators anyway, but if it was implemented it would allow us to optimize cases where we need to search for specific references repeatedly by reusing internal state. Detangle the lifecycle from iteration so that we don't deallocate the iterator anymore once it is exhausted. Instead, callers are now expected to always call a newly introduce `ref_iterator_free()` function that deallocates the iterator and its internal state. Note that the `dir_iterator` is somewhat special because it does not implement the `ref_iterator` interface, but is only used to implement other iterators. Consequently, we have to provide `dir_iterator_free()` instead of `dir_iterator_release()` as the allocated structure itself is managed by the `dir_iterator` interfaces, as well, and not freed by `ref_iterator_free()` like in all the other cases. While at it, drop the return value of `ref_iterator_abort()`, which wasn't really required by any of the iterator implementations anyway. Furthermore, stop calling `base_ref_iterator_free()` in any of the backends, but instead call it in `ref_iterator_free()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Mar 12, 2025 at 16:56 UTC cec2b6f55a805c010d2acc81abf4cbc41b712130

13 files changed +105 -186

builtin/clone.c

@@ -342,6 +342,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,

342 strbuf_setlen(src, src_len);

343 die(_("failed to iterate over '%s'"), src->buf);

344 }

345 +

346 + dir_iterator_free(iter);

347 }

348

349 static void clone_local(const char *src_repo, const char *dest_repo)

dir-iterator.c

+11 -13

@@ -193,9 +193,9 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)

193

194 if (S_ISDIR(iter->base.st.st_mode) && push_level(iter)) {

195 if (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC)

196 - goto error_out;

196 + return ITER_ERROR;

197 if (iter->levels_nr == 0)

198 - goto error_out;

198 + return ITER_ERROR;

199 }

200

201 /* Loop until we find an entry that we can give back to the caller. */

@@ -211,11 +211,11 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)

211 int ret = next_directory_entry(level->dir, iter->base.path.buf, &de);

212 if (ret < 0) {

213 if (iter->flags & DIR_ITERATOR_PEDANTIC)

214 - goto error_out;

214 + return ITER_ERROR;

215 continue;

216 } else if (ret > 0) {

217 if (pop_level(iter) == 0)

218 - return dir_iterator_abort(dir_iterator);

218 + return ITER_DONE;

219 continue;

220 }

221

@@ -223,7 +223,7 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)

223 } else {

224 if (level->entries_idx >= level->entries.nr) {

225 if (pop_level(iter) == 0)

226 - return dir_iterator_abort(dir_iterator);

226 + return ITER_DONE;

227 continue;

228 }

229

@@ -232,22 +232,21 @@ int dir_iterator_advance(struct dir_iterator *dir_iterator)

232

233 if (prepare_next_entry_data(iter, name)) {

234 if (errno != ENOENT && iter->flags & DIR_ITERATOR_PEDANTIC)

235 - goto error_out;

235 + return ITER_ERROR;

236 continue;

237 }

238

239 return ITER_OK;

240 }

241 -

242 -error_out:

243 - dir_iterator_abort(dir_iterator);

244 - return ITER_ERROR;

241 }

242

247 -int dir_iterator_abort(struct dir_iterator *dir_iterator)

243 +void dir_iterator_free(struct dir_iterator *dir_iterator)

244 {

245 struct dir_iterator_int *iter = (struct dir_iterator_int *)dir_iterator;

246

247 + if (!iter)

248 + return;

249 +

250 for (; iter->levels_nr; iter->levels_nr--) {

251 struct dir_iterator_level *level =

252 &iter->levels[iter->levels_nr - 1];

@@ -266,7 +265,6 @@ int dir_iterator_abort(struct dir_iterator *dir_iterator)

265 free(iter->levels);

266 strbuf_release(&iter->base.path);

267 free(iter);

269 - return ITER_DONE;

268 }

269

270 struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)

@@ -301,7 +299,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags)

299 return dir_iterator;

300

301 error_out:

304 - dir_iterator_abort(dir_iterator);

302 + dir_iterator_free(dir_iterator);

303 errno = saved_errno;

304 return NULL;

305 }

dir-iterator.h

+4 -7

@@ -28,7 +28,7 @@

28 *

29 * while ((ok = dir_iterator_advance(iter)) == ITER_OK) {

30 * if (want_to_stop_iteration()) {

31 - * ok = dir_iterator_abort(iter);

31 + * ok = ITER_DONE;

32 * break;

33 * }

34 *

@@ -39,6 +39,7 @@

39 *

40 * if (ok != ITER_DONE)

41 * handle_error();

42 + * dir_iterator_free(iter);

43 *

44 * Callers are allowed to modify iter->path while they are working,

45 * but they must restore it to its original contents before calling

@@ -107,11 +108,7 @@ struct dir_iterator *dir_iterator_begin(const char *path, unsigned int flags);

108 */

109 int dir_iterator_advance(struct dir_iterator *iterator);

110

110 -/*

111 - * End the iteration before it has been exhausted. Free the

112 - * dir_iterator and any associated resources and return ITER_DONE. On

113 - * error, free the dir_iterator and return ITER_ERROR.

114 - */

115 -int dir_iterator_abort(struct dir_iterator *iterator);

111 +/* Free the dir_iterator and any associated resources. */

112 +void dir_iterator_free(struct dir_iterator *iterator);

113

114 #endif

iterator.h

+1 -1

@@ -12,7 +12,7 @@

12 #define ITER_OK 0

14 /*

15 - * The iterator is exhausted and has been freed.

15 + * The iterator is exhausted.

16 */

17 #define ITER_DONE -1

refs.c

+5 -2

@@ -2485,6 +2485,7 @@ int refs_verify_refnames_available(struct ref_store *refs,

2485 struct strbuf dirname = STRBUF_INIT;

2486 struct strbuf referent = STRBUF_INIT;

2487 struct string_list_item *item;

2488 + struct ref_iterator *iter = NULL;

2489 struct strset dirnames;

2490 int ret = -1;

2491

@@ -2561,7 +2562,6 @@ int refs_verify_refnames_available(struct ref_store *refs,

2562 strbuf_addch(&dirname, '/');

2563

2564 if (!initial_transaction) {

2564 - struct ref_iterator *iter;

2565 int ok;

2566

2567 iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0,

@@ -2573,12 +2573,14 @@ int refs_verify_refnames_available(struct ref_store *refs,

2573

2574 strbuf_addf(err, _("'%s' exists; cannot create '%s'"),

2575 iter->refname, refname);

2576 - ref_iterator_abort(iter);

2576 goto cleanup;

2577 }

2578

2579 if (ok != ITER_DONE)

2580 BUG("error while iterating over references");

2581 +

2582 + ref_iterator_free(iter);

2583 + iter = NULL;

2584 }

2585

2586 extra_refname = find_descendant_ref(dirname.buf, extras, skip);

@@ -2595,6 +2597,7 @@ cleanup:

2597 strbuf_release(&referent);

2598 strbuf_release(&dirname);

2599 strset_clear(&dirnames);

2600 + ref_iterator_free(iter);

2601 return ret;

2602 }

2603

refs/debug.c

+4 -5

@@ -179,19 +179,18 @@ static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator,

179 return res;

180 }

181

182 -static int debug_ref_iterator_abort(struct ref_iterator *ref_iterator)

182 +static void debug_ref_iterator_release(struct ref_iterator *ref_iterator)

183 {

184 struct debug_ref_iterator *diter =

185 (struct debug_ref_iterator *)ref_iterator;

186 - int res = diter->iter->vtable->abort(diter->iter);

187 - trace_printf_key(&trace_refs, "iterator_abort: %d\n", res);

188 - return res;

186 + diter->iter->vtable->release(diter->iter);

187 + trace_printf_key(&trace_refs, "iterator_abort\n");

188 }

189

190 static struct ref_iterator_vtable debug_ref_iterator_vtable = {

191 .advance = debug_ref_iterator_advance,

192 .peel = debug_ref_iterator_peel,

194 - .abort = debug_ref_iterator_abort,

193 + .release = debug_ref_iterator_release,

194 };

195

196 static struct ref_iterator *

refs/files-backend.c

+10 -26

@@ -915,10 +915,6 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator)

915 return ITER_OK;

916 }

917

918 - iter->iter0 = NULL;

919 - if (ref_iterator_abort(ref_iterator) != ITER_DONE)

920 - ok = ITER_ERROR;

921 -

918 return ok;

919 }

920

@@ -931,23 +927,17 @@ static int files_ref_iterator_peel(struct ref_iterator *ref_iterator,

927 return ref_iterator_peel(iter->iter0, peeled);

928 }

929

934 -static int files_ref_iterator_abort(struct ref_iterator *ref_iterator)

930 +static void files_ref_iterator_release(struct ref_iterator *ref_iterator)

931 {

932 struct files_ref_iterator *iter =

933 (struct files_ref_iterator *)ref_iterator;

938 - int ok = ITER_DONE;

939 -

940 - if (iter->iter0)

941 - ok = ref_iterator_abort(iter->iter0);

942 -

943 - base_ref_iterator_free(ref_iterator);

944 - return ok;

934 + ref_iterator_free(iter->iter0);

935 }

936

937 static struct ref_iterator_vtable files_ref_iterator_vtable = {

938 .advance = files_ref_iterator_advance,

939 .peel = files_ref_iterator_peel,

950 - .abort = files_ref_iterator_abort,

940 + .release = files_ref_iterator_release,

941 };

942

943 static struct ref_iterator *files_ref_iterator_begin(

@@ -1378,7 +1368,7 @@ static int should_pack_refs(struct files_ref_store *refs,

1368 iter->flags, opts))

1369 refcount++;

1370 if (refcount >= limit) {

1381 - ref_iterator_abort(iter);

1371 + ref_iterator_free(iter);

1372 return 1;

1373 }

1374 }

@@ -1386,6 +1376,7 @@ static int should_pack_refs(struct files_ref_store *refs,

1376 if (ret != ITER_DONE)

1377 die("error while iterating over references");

1378

1379 + ref_iterator_free(iter);

1380 return 0;

1381 }

1382

@@ -1452,6 +1443,7 @@ static int files_pack_refs(struct ref_store *ref_store,

1443 packed_refs_unlock(refs->packed_ref_store);

1444

1445 prune_refs(refs, &refs_to_prune);

1446 + ref_iterator_free(iter);

1447 strbuf_release(&err);

1448 return 0;

1449 }

@@ -2299,9 +2291,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)

2291 return ITER_OK;

2292 }

2293

2302 - iter->dir_iterator = NULL;

2303 - if (ref_iterator_abort(ref_iterator) == ITER_ERROR)

2304 - ok = ITER_ERROR;

2294 return ok;

2295 }

2296

@@ -2311,23 +2300,17 @@ static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED,

2300 BUG("ref_iterator_peel() called for reflog_iterator");

2301 }

2302

2314 -static int files_reflog_iterator_abort(struct ref_iterator *ref_iterator)

2303 +static void files_reflog_iterator_release(struct ref_iterator *ref_iterator)

2304 {

2305 struct files_reflog_iterator *iter =

2306 (struct files_reflog_iterator *)ref_iterator;

2318 - int ok = ITER_DONE;

2319 -

2320 - if (iter->dir_iterator)

2321 - ok = dir_iterator_abort(iter->dir_iterator);

2322 -

2323 - base_ref_iterator_free(ref_iterator);

2324 - return ok;

2307 + dir_iterator_free(iter->dir_iterator);

2308 }

2309

2310 static struct ref_iterator_vtable files_reflog_iterator_vtable = {

2311 .advance = files_reflog_iterator_advance,

2312 .peel = files_reflog_iterator_peel,

2330 - .abort = files_reflog_iterator_abort,

2313 + .release = files_reflog_iterator_release,

2314 };

2315

2316 static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,

@@ -3837,6 +3820,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,

3820 ret = error(_("failed to iterate over '%s'"), sb.buf);

3821

3822 out:

3823 + dir_iterator_free(iter);

3824 strbuf_release(&sb);

3825 strbuf_release(&refname);

3826 return ret;

refs/iterator.c

+35 -65

@@ -21,9 +21,14 @@ int ref_iterator_peel(struct ref_iterator *ref_iterator,

21 return ref_iterator->vtable->peel(ref_iterator, peeled);

22 }

24 -int ref_iterator_abort(struct ref_iterator *ref_iterator)

24 +void ref_iterator_free(struct ref_iterator *ref_iterator)

25 {

26 - return ref_iterator->vtable->abort(ref_iterator);

26 + if (ref_iterator) {

27 + ref_iterator->vtable->release(ref_iterator);

28 + /* Help make use-after-free bugs fail quickly: */

29 + ref_iterator->vtable = NULL;

30 + free(ref_iterator);

31 + }

32 }

34 void base_ref_iterator_init(struct ref_iterator *iter,

@@ -36,20 +41,13 @@ void base_ref_iterator_init(struct ref_iterator *iter,

41 iter->flags = 0;

42 }

39 -void base_ref_iterator_free(struct ref_iterator *iter)

40 -{

41 - /* Help make use-after-free bugs fail quickly: */

42 - iter->vtable = NULL;

43 - free(iter);

44 -}

45 -

44 struct empty_ref_iterator {

45 struct ref_iterator base;

46 };

50 -static int empty_ref_iterator_advance(struct ref_iterator *ref_iterator)

48 +static int empty_ref_iterator_advance(struct ref_iterator *ref_iterator UNUSED)

49 {

52 - return ref_iterator_abort(ref_iterator);

50 + return ITER_DONE;

51 }

53 static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator UNUSED,

@@ -58,16 +56,14 @@ static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator UNUSED,

56 BUG("peel called for empty iterator");

57 }

61 -static int empty_ref_iterator_abort(struct ref_iterator *ref_iterator)

59 +static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED)

60 {

63 - base_ref_iterator_free(ref_iterator);

64 - return ITER_DONE;

61 }

63 static struct ref_iterator_vtable empty_ref_iterator_vtable = {

64 .advance = empty_ref_iterator_advance,

65 .peel = empty_ref_iterator_peel,

70 - .abort = empty_ref_iterator_abort,

66 + .release = empty_ref_iterator_release,

67 };

69 struct ref_iterator *empty_ref_iterator_begin(void)

@@ -151,11 +147,13 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator)

147 if (!iter->current) {

148 /* Initialize: advance both iterators to their first entries */

149 if ((ok = ref_iterator_advance(iter->iter0)) != ITER_OK) {

150 + ref_iterator_free(iter->iter0);

151 iter->iter0 = NULL;

152 if (ok == ITER_ERROR)

153 goto error;

154 }

155 if ((ok = ref_iterator_advance(iter->iter1)) != ITER_OK) {

156 + ref_iterator_free(iter->iter1);

157 iter->iter1 = NULL;

158 if (ok == ITER_ERROR)

159 goto error;

@@ -166,6 +164,7 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator)

164 * entry:

165 */

166 if ((ok = ref_iterator_advance(*iter->current)) != ITER_OK) {

167 + ref_iterator_free(*iter->current);

168 *iter->current = NULL;

169 if (ok == ITER_ERROR)

170 goto error;

@@ -179,9 +178,8 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator)

178 iter->select(iter->iter0, iter->iter1, iter->cb_data);

179

180 if (selection == ITER_SELECT_DONE) {

182 - return ref_iterator_abort(ref_iterator);

181 + return ITER_DONE;

182 } else if (selection == ITER_SELECT_ERROR) {

184 - ref_iterator_abort(ref_iterator);

183 return ITER_ERROR;

184 }

185

@@ -195,6 +193,7 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator)

193

194 if (selection & ITER_SKIP_SECONDARY) {

195 if ((ok = ref_iterator_advance(*secondary)) != ITER_OK) {

196 + ref_iterator_free(*secondary);

197 *secondary = NULL;

198 if (ok == ITER_ERROR)

199 goto error;

@@ -211,7 +210,6 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator)

210 }

211

212 error:

214 - ref_iterator_abort(ref_iterator);

213 return ITER_ERROR;

214 }

215

@@ -227,28 +225,18 @@ static int merge_ref_iterator_peel(struct ref_iterator *ref_iterator,

225 return ref_iterator_peel(*iter->current, peeled);

226 }

227

230 -static int merge_ref_iterator_abort(struct ref_iterator *ref_iterator)

228 +static void merge_ref_iterator_release(struct ref_iterator *ref_iterator)

229 {

230 struct merge_ref_iterator *iter =

231 (struct merge_ref_iterator *)ref_iterator;

234 - int ok = ITER_DONE;

235 -

236 - if (iter->iter0) {

237 - if (ref_iterator_abort(iter->iter0) != ITER_DONE)

238 - ok = ITER_ERROR;

239 - }

240 - if (iter->iter1) {

241 - if (ref_iterator_abort(iter->iter1) != ITER_DONE)

242 - ok = ITER_ERROR;

243 - }

244 - base_ref_iterator_free(ref_iterator);

245 - return ok;

232 + ref_iterator_free(iter->iter0);

233 + ref_iterator_free(iter->iter1);

234 }

235

236 static struct ref_iterator_vtable merge_ref_iterator_vtable = {

237 .advance = merge_ref_iterator_advance,

238 .peel = merge_ref_iterator_peel,

251 - .abort = merge_ref_iterator_abort,

239 + .release = merge_ref_iterator_release,

240 };

241

242 struct ref_iterator *merge_ref_iterator_begin(

@@ -310,10 +298,10 @@ struct ref_iterator *overlay_ref_iterator_begin(

298 * them.

299 */

300 if (is_empty_ref_iterator(front)) {

313 - ref_iterator_abort(front);

301 + ref_iterator_free(front);

302 return back;

303 } else if (is_empty_ref_iterator(back)) {

316 - ref_iterator_abort(back);

304 + ref_iterator_free(back);

305 return front;

306 }

307

@@ -350,19 +338,15 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator)

338

339 while ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) {

340 int cmp = compare_prefix(iter->iter0->refname, iter->prefix);

353 -

341 if (cmp < 0)

342 continue;

356 -

357 - if (cmp > 0) {

358 - /*

359 - * As the source iterator is ordered, we

360 - * can stop the iteration as soon as we see a

361 - * refname that comes after the prefix:

362 - */

363 - ok = ref_iterator_abort(iter->iter0);

364 - break;

365 - }

343 + /*

344 + * As the source iterator is ordered, we

345 + * can stop the iteration as soon as we see a

346 + * refname that comes after the prefix:

347 + */

348 + if (cmp > 0)

349 + return ITER_DONE;

350

351 if (iter->trim) {

352 /*

@@ -386,9 +370,6 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator)

370 return ITER_OK;

371 }

372

389 - iter->iter0 = NULL;

390 - if (ref_iterator_abort(ref_iterator) != ITER_DONE)

391 - return ITER_ERROR;

373 return ok;

374 }

375

@@ -401,23 +382,18 @@ static int prefix_ref_iterator_peel(struct ref_iterator *ref_iterator,

382 return ref_iterator_peel(iter->iter0, peeled);

383 }

384

404 -static int prefix_ref_iterator_abort(struct ref_iterator *ref_iterator)

385 +static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator)

386 {

387 struct prefix_ref_iterator *iter =

388 (struct prefix_ref_iterator *)ref_iterator;

408 - int ok = ITER_DONE;

409 -

410 - if (iter->iter0)

411 - ok = ref_iterator_abort(iter->iter0);

389 + ref_iterator_free(iter->iter0);

390 free(iter->prefix);

413 - base_ref_iterator_free(ref_iterator);

414 - return ok;

391 }

392

393 static struct ref_iterator_vtable prefix_ref_iterator_vtable = {

394 .advance = prefix_ref_iterator_advance,

395 .peel = prefix_ref_iterator_peel,

420 - .abort = prefix_ref_iterator_abort,

396 + .release = prefix_ref_iterator_release,

397 };

398

399 struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,

@@ -453,20 +429,14 @@ int do_for_each_ref_iterator(struct ref_iterator *iter,

429 current_ref_iter = iter;

430 while ((ok = ref_iterator_advance(iter)) == ITER_OK) {

431 retval = fn(iter->refname, iter->referent, iter->oid, iter->flags, cb_data);

456 - if (retval) {

457 - /*

458 - * If ref_iterator_abort() returns ITER_ERROR,

459 - * we ignore that error in deference to the

460 - * callback function's return value.

461 - */

462 - ref_iterator_abort(iter);

432 + if (retval)

433 goto out;

464 - }

434 }

435

436 out:

437 current_ref_iter = old_ref_iter;

438 if (ok == ITER_ERROR)

470 - return -1;

439 + retval = -1;

440 + ref_iterator_free(iter);

441 return retval;

442 }

refs/packed-backend.c

+12 -15

@@ -954,9 +954,6 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator)

954 return ITER_OK;

955 }

956

957 - if (ref_iterator_abort(ref_iterator) != ITER_DONE)

958 - ok = ITER_ERROR;

959 -

957 return ok;

958 }

959

@@ -976,23 +973,19 @@ static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator,

973 }

974 }

975

979 -static int packed_ref_iterator_abort(struct ref_iterator *ref_iterator)

976 +static void packed_ref_iterator_release(struct ref_iterator *ref_iterator)

977 {

978 struct packed_ref_iterator *iter =

979 (struct packed_ref_iterator *)ref_iterator;

983 - int ok = ITER_DONE;

984 -

980 strbuf_release(&iter->refname_buf);

981 free(iter->jump);

982 release_snapshot(iter->snapshot);

988 - base_ref_iterator_free(ref_iterator);

989 - return ok;

983 }

984

985 static struct ref_iterator_vtable packed_ref_iterator_vtable = {

986 .advance = packed_ref_iterator_advance,

987 .peel = packed_ref_iterator_peel,

995 - .abort = packed_ref_iterator_abort

988 + .release = packed_ref_iterator_release,

989 };

990

991 static int jump_list_entry_cmp(const void *va, const void *vb)

@@ -1362,8 +1355,10 @@ static int write_with_updates(struct packed_ref_store *refs,

1355 */

1356 iter = packed_ref_iterator_begin(&refs->base, "", NULL,

1357 DO_FOR_EACH_INCLUDE_BROKEN);

1365 - if ((ok = ref_iterator_advance(iter)) != ITER_OK)

1358 + if ((ok = ref_iterator_advance(iter)) != ITER_OK) {

1359 + ref_iterator_free(iter);

1360 iter = NULL;

1361 + }

1362

1363 i = 0;

1364

@@ -1411,8 +1406,10 @@ static int write_with_updates(struct packed_ref_store *refs,

1406 * the iterator over the unneeded

1407 * value.

1408 */

1414 - if ((ok = ref_iterator_advance(iter)) != ITER_OK)

1409 + if ((ok = ref_iterator_advance(iter)) != ITER_OK) {

1410 + ref_iterator_free(iter);

1411 iter = NULL;

1412 + }

1413 cmp = +1;

1414 } else {

1415 /*

@@ -1449,8 +1446,10 @@ static int write_with_updates(struct packed_ref_store *refs,

1446 peel_error ? NULL : &peeled))

1447 goto write_error;

1448

1452 - if ((ok = ref_iterator_advance(iter)) != ITER_OK)

1449 + if ((ok = ref_iterator_advance(iter)) != ITER_OK) {

1450 + ref_iterator_free(iter);

1451 iter = NULL;

1452 + }

1453 } else if (is_null_oid(&update->new_oid)) {

1454 /*

1455 * The update wants to delete the reference,

@@ -1499,9 +1498,7 @@ write_error:

1498 get_tempfile_path(refs->tempfile), strerror(errno));

1499

1500 error:

1502 - if (iter)

1503 - ref_iterator_abort(iter);

1504 -

1501 + ref_iterator_free(iter);

1502 delete_tempfile(&refs->tempfile);

1503 return -1;

1504 }

refs/ref-cache.c

+3 -6

@@ -409,7 +409,7 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)

409 if (++level->index == level->dir->nr) {

410 /* This level is exhausted; pop up a level */

411 if (--iter->levels_nr == 0)

412 - return ref_iterator_abort(ref_iterator);

412 + return ITER_DONE;

413

414 continue;

415 }

@@ -452,21 +452,18 @@ static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator,

452 return peel_object(iter->repo, ref_iterator->oid, peeled) ? -1 : 0;

453 }

454

455 -static int cache_ref_iterator_abort(struct ref_iterator *ref_iterator)

455 +static void cache_ref_iterator_release(struct ref_iterator *ref_iterator)

456 {

457 struct cache_ref_iterator *iter =

458 (struct cache_ref_iterator *)ref_iterator;

459 -

459 free((char *)iter->prefix);

460 free(iter->levels);

462 - base_ref_iterator_free(ref_iterator);

463 - return ITER_DONE;

461 }

462

463 static struct ref_iterator_vtable cache_ref_iterator_vtable = {

464 .advance = cache_ref_iterator_advance,

465 .peel = cache_ref_iterator_peel,

469 - .abort = cache_ref_iterator_abort

466 + .release = cache_ref_iterator_release,

467 };

468

469 struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,

refs/refs-internal.h

+9 -20

@@ -273,11 +273,11 @@ enum do_for_each_ref_flags {

273 * the next reference and returns ITER_OK. The data pointed at by

274 * refname and oid belong to the iterator; if you want to retain them

275 * after calling ref_iterator_advance() again or calling

276 - * ref_iterator_abort(), you must make a copy. When the iteration has

276 + * ref_iterator_free(), you must make a copy. When the iteration has

277 * been exhausted, ref_iterator_advance() releases any resources

278 * associated with the iteration, frees the ref_iterator object, and

279 * returns ITER_DONE. If you want to abort the iteration early, call

280 - * ref_iterator_abort(), which also frees the ref_iterator object and

280 + * ref_iterator_free(), which also frees the ref_iterator object and

281 * any associated resources. If there was an internal error advancing

282 * to the next entry, ref_iterator_advance() aborts the iteration,

283 * frees the ref_iterator, and returns ITER_ERROR.

@@ -293,7 +293,7 @@ enum do_for_each_ref_flags {

293 *

294 * while ((ok = ref_iterator_advance(iter)) == ITER_OK) {

295 * if (want_to_stop_iteration()) {

296 - * ok = ref_iterator_abort(iter);

296 + * ok = ITER_DONE;

297 * break;

298 * }

299 *

@@ -307,6 +307,7 @@ enum do_for_each_ref_flags {

307 *

308 * if (ok != ITER_DONE)

309 * handle_error();

310 + * ref_iterator_free(iter);

311 */

312 struct ref_iterator {

313 struct ref_iterator_vtable *vtable;

@@ -333,12 +334,8 @@ int ref_iterator_advance(struct ref_iterator *ref_iterator);

334 int ref_iterator_peel(struct ref_iterator *ref_iterator,

335 struct object_id *peeled);

336

336 -/*

337 - * End the iteration before it has been exhausted, freeing the

338 - * reference iterator and any associated resources and returning

339 - * ITER_DONE. If the abort itself failed, return ITER_ERROR.

340 - */

341 -int ref_iterator_abort(struct ref_iterator *ref_iterator);

337 +/* Free the reference iterator and any associated resources. */

338 +void ref_iterator_free(struct ref_iterator *ref_iterator);

339

340 /*

341 * An iterator over nothing (its first ref_iterator_advance() call

@@ -438,13 +435,6 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,

435 void base_ref_iterator_init(struct ref_iterator *iter,

436 struct ref_iterator_vtable *vtable);

437

441 -/*

442 - * Base class destructor for ref_iterators. Destroy the ref_iterator

443 - * part of iter and shallow-free the object. This is meant to be

444 - * called only by the destructors of derived classes.

445 - */

446 -void base_ref_iterator_free(struct ref_iterator *iter);

447 -

438 /* Virtual function declarations for ref_iterators: */

439

440 /*

@@ -463,15 +453,14 @@ typedef int ref_iterator_peel_fn(struct ref_iterator *ref_iterator,

453

454 /*

455 * Implementations of this function should free any resources specific

466 - * to the derived class, then call base_ref_iterator_free() to clean

467 - * up and free the ref_iterator object.

456 + * to the derived class.

457 */

469 -typedef int ref_iterator_abort_fn(struct ref_iterator *ref_iterator);

458 +typedef void ref_iterator_release_fn(struct ref_iterator *ref_iterator);

459

460 struct ref_iterator_vtable {

461 ref_iterator_advance_fn *advance;

462 ref_iterator_peel_fn *peel;

474 - ref_iterator_abort_fn *abort;

463 + ref_iterator_release_fn *release;

464 };

465

466 /*

refs/reftable-backend.c

+8 -26

@@ -711,17 +711,10 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)

711 break;

712 }

713

714 - if (iter->err > 0) {

715 - if (ref_iterator_abort(ref_iterator) != ITER_DONE)

716 - return ITER_ERROR;

714 + if (iter->err > 0)

715 return ITER_DONE;

718 - }

719 -

720 - if (iter->err < 0) {

721 - ref_iterator_abort(ref_iterator);

716 + if (iter->err < 0)

717 return ITER_ERROR;

723 - }

724 -

718 return ITER_OK;

719 }

720

@@ -740,7 +733,7 @@ static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator,

733 return -1;

734 }

735

743 -static int reftable_ref_iterator_abort(struct ref_iterator *ref_iterator)

736 +static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator)

737 {

738 struct reftable_ref_iterator *iter =

739 (struct reftable_ref_iterator *)ref_iterator;

@@ -751,14 +744,12 @@ static int reftable_ref_iterator_abort(struct ref_iterator *ref_iterator)

744 free(iter->exclude_patterns[i]);

745 free(iter->exclude_patterns);

746 }

754 - free(iter);

755 - return ITER_DONE;

747 }

748

749 static struct ref_iterator_vtable reftable_ref_iterator_vtable = {

750 .advance = reftable_ref_iterator_advance,

751 .peel = reftable_ref_iterator_peel,

761 - .abort = reftable_ref_iterator_abort

752 + .release = reftable_ref_iterator_release,

753 };

754

755 static int qsort_strcmp(const void *va, const void *vb)

@@ -2020,17 +2011,10 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)

2011 break;

2012 }

2013

2023 - if (iter->err > 0) {

2024 - if (ref_iterator_abort(ref_iterator) != ITER_DONE)

2025 - return ITER_ERROR;

2014 + if (iter->err > 0)

2015 return ITER_DONE;

2027 - }

2028 -

2029 - if (iter->err < 0) {

2030 - ref_iterator_abort(ref_iterator);

2016 + if (iter->err < 0)

2017 return ITER_ERROR;

2032 - }

2033 -

2018 return ITER_OK;

2019 }

2020

@@ -2041,21 +2025,19 @@ static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSE

2025 return -1;

2026 }

2027

2044 -static int reftable_reflog_iterator_abort(struct ref_iterator *ref_iterator)

2028 +static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator)

2029 {

2030 struct reftable_reflog_iterator *iter =

2031 (struct reftable_reflog_iterator *)ref_iterator;

2032 reftable_log_record_release(&iter->log);

2033 reftable_iterator_destroy(&iter->iter);

2034 strbuf_release(&iter->last_name);

2051 - free(iter);

2052 - return ITER_DONE;

2035 }

2036

2037 static struct ref_iterator_vtable reftable_reflog_iterator_vtable = {

2038 .advance = reftable_reflog_iterator_advance,

2039 .peel = reftable_reflog_iterator_peel,

2058 - .abort = reftable_reflog_iterator_abort

2040 + .release = reftable_reflog_iterator_release,

2041 };

2042

2043 static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftable_ref_store *refs,

t/helper/test-dir-iterator.c

@@ -53,6 +53,7 @@ int cmd__dir_iterator(int argc, const char **argv)

53 printf("(%s) [%s] %s\n", diter->relative_path, diter->basename,

54 diter->path.buf);

55 }

56 + dir_iterator_free(diter);

58 if (iter_status != ITER_DONE) {

59 printf("dir_iterator_advance failure\n");