reftable/stack: refactor stack reloading to have common exit path

The `reftable_stack_reload_maybe_reuse()` function is responsible for reloading the reftable list from disk. The function is quite hard to follow though because it has a bunch of different exit paths, many of which have to free the same set of resources. Refactor the function to have a common exit path. While at it, touch up the style of this function a bit to match our usual coding style better. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jan 11, 2024 at 11:06 UTC 3c94bd8dfb43d1e6f38184e34f4d5a0046dd00d7
1 file changed +42 -44
reftable/stack.c
+42 -44
@@ -304,69 +304,67 @@ static int tv_cmp(struct timeval *a, struct timeval *b)
304 static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
305 int reuse_open)
306 {
307 - struct timeval deadline = { 0 };
308 - int err = gettimeofday(&deadline, NULL);
307 + char **names = NULL, **names_after = NULL;
308 + struct timeval deadline;
309 int64_t delay = 0;
310 - int tries = 0;
311 - if (err < 0)
312 - return err;
310 + int tries = 0, err;
311
312 + err = gettimeofday(&deadline, NULL);
313 + if (err < 0)
314 + goto out;
315 deadline.tv_sec += 3;
316 +
317 while (1) {
316 - char **names = NULL;
317 - char **names_after = NULL;
318 - struct timeval now = { 0 };
319 - int err = gettimeofday(&now, NULL);
320 - int err2 = 0;
321 - if (err < 0) {
322 - return err;
323 - }
318 + struct timeval now;
319
325 - /* Only look at deadlines after the first few times. This
326 - simplifies debugging in GDB */
320 + err = gettimeofday(&now, NULL);
321 + if (err < 0)
322 + goto out;
323 +
324 + /*
325 + * Only look at deadlines after the first few times. This
326 + * simplifies debugging in GDB.
327 + */
328 tries++;
328 - if (tries > 3 && tv_cmp(&now, &deadline) >= 0) {
329 - break;
330 - }
329 + if (tries > 3 && tv_cmp(&now, &deadline) >= 0)
330 + goto out;
331
332 err = read_lines(st->list_file, &names);
333 - if (err < 0) {
334 - free_names(names);
335 - return err;
336 - }
333 + if (err < 0)
334 + goto out;
335 +
336 err = reftable_stack_reload_once(st, names, reuse_open);
338 - if (err == 0) {
339 - free_names(names);
337 + if (!err)
338 break;
341 - }
342 - if (err != REFTABLE_NOT_EXIST_ERROR) {
343 - free_names(names);
344 - return err;
345 - }
346 -
347 - /* err == REFTABLE_NOT_EXIST_ERROR can be caused by a concurrent
348 - writer. Check if there was one by checking if the name list
349 - changed.
350 - */
351 - err2 = read_lines(st->list_file, &names_after);
352 - if (err2 < 0) {
353 - free_names(names);
354 - return err2;
355 - }
356 -
339 + if (err != REFTABLE_NOT_EXIST_ERROR)
340 + goto out;
341 +
342 + /*
343 + * REFTABLE_NOT_EXIST_ERROR can be caused by a concurrent
344 + * writer. Check if there was one by checking if the name list
345 + * changed.
346 + */
347 + err = read_lines(st->list_file, &names_after);
348 + if (err < 0)
349 + goto out;
350 if (names_equal(names_after, names)) {
358 - free_names(names);
359 - free_names(names_after);
360 - return err;
351 + err = REFTABLE_NOT_EXIST_ERROR;
352 + goto out;
353 }
354 +
355 free_names(names);
356 + names = NULL;
357 free_names(names_after);
358 + names_after = NULL;
359
360 delay = delay + (delay * rand()) / RAND_MAX + 1;
361 sleep_millisec(delay);
362 }
363
369 - return 0;
364 +out:
365 + free_names(names);
366 + free_names(names_after);
367 + return err;
368 }
369
370 /* -1 = error