reftable/merged: refactor seeking of records

The code to seek reftable records in the merged table code is quite hard to read and does not conform to our coding style in multiple ways: - We have multiple exit paths where we release resources even though that is not really necessary. - We use a scoped error variable `e` which is hard to reason about. This variable is not required at all. - We allocate memory in the variable declarations, which is easy to miss. Refactor the function so that it becomes more maintainable in the future. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 6, 2024 at 07:35 UTC 59f302ca5abf3e8ec4f14f098b26adf786017fad
1 file changed +21 -33
reftable/merged.c
+21 -33
@@ -238,50 +238,38 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
238 struct reftable_iterator *it,
239 struct reftable_record *rec)
240 {
241 - struct reftable_iterator *iters = reftable_calloc(
242 - mt->stack_len, sizeof(*iters));
241 struct merged_iter merged = {
244 - .stack = iters,
242 .typ = reftable_record_type(rec),
243 .hash_id = mt->hash_id,
244 .suppress_deletions = mt->suppress_deletions,
245 .key = STRBUF_INIT,
246 .entry_key = STRBUF_INIT,
247 };
251 - int n = 0;
252 - int err = 0;
253 - int i = 0;
254 - for (i = 0; i < mt->stack_len && err == 0; i++) {
255 - int e = reftable_table_seek_record(&mt->stack[i], &iters[n],
256 - rec);
257 - if (e < 0) {
258 - err = e;
259 - }
260 - if (e == 0) {
261 - n++;
262 - }
263 - }
264 - if (err < 0) {
265 - int i = 0;
266 - for (i = 0; i < n; i++) {
267 - reftable_iterator_destroy(&iters[i]);
268 - }
269 - reftable_free(iters);
270 - return err;
248 + struct merged_iter *p;
249 + int err;
250 +
251 + REFTABLE_CALLOC_ARRAY(merged.stack, mt->stack_len);
252 + for (size_t i = 0; i < mt->stack_len; i++) {
253 + err = reftable_table_seek_record(&mt->stack[i],
254 + &merged.stack[merged.stack_len], rec);
255 + if (err < 0)
256 + goto out;
257 + if (!err)
258 + merged.stack_len++;
259 }
260
273 - merged.stack_len = n;
261 err = merged_iter_init(&merged);
275 - if (err < 0) {
262 + if (err < 0)
263 + goto out;
264 +
265 + p = reftable_malloc(sizeof(struct merged_iter));
266 + *p = merged;
267 + iterator_from_merged_iter(it, p);
268 +
269 +out:
270 + if (err < 0)
271 merged_iter_close(&merged);
277 - return err;
278 - } else {
279 - struct merged_iter *p =
280 - reftable_malloc(sizeof(struct merged_iter));
281 - *p = merged;
282 - iterator_from_merged_iter(it, p);
283 - }
284 - return 0;
272 + return err;
273 }
274
275 int reftable_merged_table_seek_ref(struct reftable_merged_table *mt,