[PATCH 1/7] chunk-format: introduce `pair_chunk_expect()` helper
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Nov 9, 2023, 22:34 UTC
- Message-ID
- <af5fe3b7237caeba8f970e967933db96c83a230e.1699569246.git.me@ttaylorr.com>
- In-Reply-To
- <cover.1699569246.git.me@ttaylorr.com>
In 570b8b8836 (chunk-format: note that pair_chunk() is unsafe, 2023-10-09), the pair_chunk() interface grew a required "size" pointer, so that the caller is forced to at least have a handle on the actual size of the given chunk.
Many callers were converted to the new interface. A handful were instead converted from the unsafe version of pair_chunk() to read_chunk() so that they could check their expected size.
This led to a lot of code like:
static int graph_read_oid_lookup(const unsigned char *chunk_start,
size_t chunk_size, void *data)
{
struct commit_graph *g = data;
g->chunk_oid_lookup = chunk_start;
if (chunk_size / g->hash_len != g->num_commits)
return error(_("commit-graph OID lookup chunk is the wrong size"));
return 0;
}, leaving the caller to use read_chunk(), like so:
read_chunk(cf, GRAPH_CHUNKID_OIDLOOKUP, graph_read_oid_lookup, graph);
The callback to read_chunk() (in the above, `graph_read_oid_lookup()`) does nothing more than (a) assign a pointer to the location of the start of the chunk in the mmap'd file, and (b) assert that it has the correct size.
For callers that know the expected size of their chunk(s) up-front (most often because they are made up of a known number of fixed-size records), we can simplify this by teaching the chunk-format API itself to validate the expected size for us.
This is wrapped in a new function, called `pair_chunk_expect()` which takes a pair of "size_t"s (corresponding to the record size and count), instead of a "size_t *", and validates that the given chunk matches the expected size as given.
This will allow us to reduce the number of lines of code it takes to perform these basic read_chunk() operations, by taking the above and replacing it with something like:
if (pair_chunk_expect(cf, GRAPH_CHUNKID_OIDLOOKUP,
&graph->chunk_oid_lookup,
graph->hash_len, graph->num_commits))
error(_("commit-graph oid lookup chunk is wrong size"));We will perform those transformations in the following commits.
Helped-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- chunk-format.c | 29 +++++++++++++++++++++++++++++ chunk-format.h | 13 ++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/chunk-format.c b/chunk-format.c index cdc7f39b70..be078dcca8 100644 --- a/chunk-format.c +++ b/chunk-format.c @@ -163,6 +163,10 @@ int read_table_of_contents(struct chunkfile *cf, struct pair_chunk_data { const unsigned char **p; size_t *size; + + /* for pair_chunk_expect() only */ + size_t record_size; + size_t record_nr; }; static int pair_chunk_fn(const unsigned char *chunk_start, @@ -175,6 +179,17 @@ static int pair_chunk_fn(const unsigned char *chunk_start, return 0; } +static int pair_chunk_expect_fn(const unsigned char *chunk_start, + size_t chunk_size, + void *data) +{ + struct pair_chunk_data *pcd = data; + if (chunk_size / pcd->record_size != pcd->record_nr) + return -1; + *pcd->p = chunk_start; + return 0; +} + int pair_chunk(struct chunkfile *cf, uint32_t chunk_id, const unsigned char **p, @@ -184,6 +199,20 @@ int pair_chunk(struct chunkfile *cf, return read_chunk(cf, chunk_id, pair_chunk_fn, &pcd); } +int pair_chunk_expect(struct chunkfile *cf, + uint32_t chunk_id, + const unsigned char **p, + size_t record_size, + size_t record_nr) +{ + struct pair_chunk_data pcd = { + .p = p, + .record_size = record_size, + .record_nr = record_nr, + }; + return read_chunk(cf, chunk_id, pair_chunk_expect_fn, &pcd); +} + int read_chunk(struct chunkfile *cf, uint32_t chunk_id, chunk_read_fn fn, diff --git a/chunk-format.h b/chunk-format.h index 14b76180ef..10806d7a9a 100644 --- a/chunk-format.h +++ b/chunk-format.h @@ -17,7 +17,8 @@ struct chunkfile; * * If reading a file, use a NULL 'struct hashfile *' and then call * read_table_of_contents(). Supply the memory-mapped data to the - * pair_chunk() or read_chunk() methods, as appropriate. + * pair_chunk(), pair_chunk_expect(), or read_chunk() methods, as + * appropriate. * * DO NOT MIX THESE MODES. Use different 'struct chunkfile' instances * for reading and writing. @@ -54,6 +55,16 @@ int pair_chunk(struct chunkfile *cf, const unsigned char **p, size_t *size); +/* + * Similar to 'pair_chunk', but used for callers who are reading a chunk + * with a known number of fixed-width records. + */ +int pair_chunk_expect(struct chunkfile *cf, + uint32_t chunk_id, + const unsigned char **p, + size_t record_size, + size_t record_nr); + typedef int (*chunk_read_fn)(const unsigned char *chunk_start, size_t chunk_size, void *data); /*
-- 2.43.0.rc0.39.g44bd344727