Subject: Re: [PATCH 1/4] t/helper/test-bitmap.c: add 'dump-hashes' mode

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 08 Sep 2021 03:37:05 +0200

On Tue, Sep 07 2021, Taylor Blau wrote:

> +int test_bitmap_hashes(struct repository *r)
> +{
> +	struct bitmap_index *bitmap_git = prepare_bitmap_git(r);
> +	struct object_id oid;
> +	uint32_t i;
> +
> +	if (!bitmap_git->hashes)
> +		goto cleanup;
> +
> +	for (i = 0; i < bitmap_num_objects(bitmap_git); i++) {
> +		nth_bitmap_object_oid(bitmap_git, &oid, i);
> +
> +		printf("%s %"PRIu32"\n",
> +		       oid_to_hex(&oid), get_be32(bitmap_git->hashes + i));
> +	}
> +
> +cleanup:
> +	free_bitmap_index(bitmap_git);
> +
> +	return 0;
> +}

Seems like it should return void, it doesn't ever return non-0, nor in
the rest of this series....

> +static int bitmap_dump_hashes(void)
> +{
> +	return test_bitmap_hashes(the_repository);
> +}
> [...]
>  		return bitmap_list_commits();
> +	if (!strcmp(argv[1], "dump-hashes"))
> +		return bitmap_dump_hashes();

Perhaps the return code only for the brevity of this test-only code?
Seems like having bitmap_dump_hashes() do the "return 0" would be better
in that case.