On Tue, Aug 19, 2025 at 02:21:02PM +0200, Karthik Nayak wrote:
> diff --git a/reftable/fsck.c b/reftable/fsck.c
> index 22ec3c26e9..e92a630276 100644
> --- a/reftable/fsck.c
> +++ b/reftable/fsck.c
> @@ -2,6 +2,28 @@
> #include "reftable-fsck.h"
> #include "stack.h"
>
> +static int reftable_fsck_valid_stack_count(struct reftable_stack *st)
> +{
> + DIR *dir = opendir(st->reftable_dir);
> + struct dirent *d = NULL;
> + unsigned int count = 0;
> +
> + if (!dir)
> + return 0;
> +
> + while ((d = readdir(dir))) {
> + if (!strcmp(d->d_name, "tables.list"))
> + continue;
> +
> + if (d->d_type == DT_REG)
> + count++;
> + }
> +
> + closedir(dir);
> +
> + return count == st->tables_len;
> +}
> +
The above logic is clear to understand but I think we should our
internal interface in "dir-iterator.h" to implement above logic.
> int reftable_fsck_check(struct reftable_stack *stack,
> reftable_fsck_report_fn report_fn,
> reftable_fsck_verbose_fn verbose_fn,
> @@ -44,6 +66,18 @@ int reftable_fsck_check(struct reftable_stack *stack,
> }
> }
>
> + verbose_fn("Checking reftable tables count", cb_data);
> +
> + if (!reftable_fsck_valid_stack_count(stack)) {
> + struct reftable_fsck_info info = {
> + .error = REFTABLE_FSCK_ERROR_STACK_COUNT,
> + .path = stack->list_file,
> + .msg = "mismatch in number of tables"
> + };
> +
When reading here, I somehow understand the reason why you define this
data structure in the loop. But I still think we could just define only
one `info`.
BTY, I wonder whether we should define some auxiliary functions for each
check instead of adding logic directly in `reftable_fsck_check`
function?
> + err = report_fn(info, cb_data);
> + }
> +
> out:
> free_names(names);
> return err;
Thanks,
Jialuo
