Re: [PATCH 14/17] midx: use chunk-format read API

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 27, 2021, 03:06 UTC
Message-ID: <YBDY0Me2QMcUMD2e@nand.local>
In-Reply-To: <cb145e0e32afed99b9bfa822c76f48bee18885ba.1611676886.git.gitgitgadget@gmail.com>

On Tue, Jan 26, 2021 at 04:01:23PM +0000, Derrick Stolee via GitGitGadget wrote:

Show 28 quoted lines

> From: Derrick Stolee <dstolee@microsoft.com>
>
> Instead of parsing the table of contents directly, use the chunk-format
> API methods read_table_of_contents() and pair_chunk(). In particular, we
> can use the return value of pair_chunk() to generate an error when a
> required chunk is missing.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  midx.c                      | 103 ++++++++++++++++++++----------------
>  t/t5319-multi-pack-index.sh |   6 +--
>  2 files changed, 60 insertions(+), 49 deletions(-)
>
> diff --git a/midx.c b/midx.c
> index 0bfd2d802b6..dd019c00795 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -54,6 +54,51 @@ static char *get_midx_filename(const char *object_dir)
>  	return xstrfmt("%s/pack/multi-pack-index", object_dir);
>  }
>
> +static int midx_read_pack_names(const unsigned char *chunk_start,
> +				size_t chunk_size, void *data)
> +{
> +	struct multi_pack_index *m = (struct multi_pack_index *)data;
> +	m->chunk_pack_names = chunk_start;
> +	return 0;
> +}

There are a lot of these callbacks that just assign some 'void **' to point at chunk_start.

Maybe a good use of the "pair_chunk" name would be something like:

    int pair_chunk(struct chunkfile *cf, uint32_t id, const unsigned char **p);

which does the same as what you wrote here. So instead of what you wrote, you could instead:

    pair_chunk(cf, MIDX_CHUNKID_PACKNAMES, &m->chunk_pack_names);

This would be in addition to the richer callback-style function which allows the caller greater flexibility (e.g., for the Bloom filter related readers in the commit-graph code).

Thanks, Taylor