[PATCH 1/8] chunk-format: introduce `pair_chunk_expect()` helper

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 13, 2023, 19:25 UTC
Message-ID: <6fa20034b2246599b3e1cd49accfc421b0fba4fb.1697225110.git.me@ttaylorr.com>
In-Reply-To: <cover.1697225110.git.me@ttaylorr.com>

In previous commits, 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_fanout(const unsigned char *chunk_start,
                                     size_t chunk_size, void *data)
    {
      struct commit_graph *g = data;
      if (chunk_size != 256 * sizeof(uint32_t))
        return error("commit-graph oid fanout chunk is wrong size");
      g->chunk_oid_fanout = (const uint32_t *)chunk_start;
      return 0;
    }

, leaving the caller to use read_chunk(), like so:

    read_chunk(cf, GRAPH_CHUNKID_OIDFANOUT, graph_read_oid_fanout, graph);

The callback to read_chunk() (in the above, `graph_read_oid_fanout()`) 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, 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 "size_t" 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_OIDFANOUT,
                          (const unsigned char **)&graph->chunk_oid_fanout,
                          256 * sizeof(uint32_t)))
      error(_("commit-graph oid fanout chunk is wrong size"));

We will perform those transformations in the following commits.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 chunk-format.c | 22 ++++++++++++++++++++++
 chunk-format.h | 12 +++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/chunk-format.c b/chunk-format.c
index cdc7f39b70..9550f15699 100644
--- a/chunk-format.c
+++ b/chunk-format.c
@@ -163,6 +163,8 @@ int read_table_of_contents(struct chunkfile *cf,
 struct pair_chunk_data {
 	const unsigned char **p;
 	size_t *size;
+
+	size_t expected_size;
 };
 
 static int pair_chunk_fn(const unsigned char *chunk_start,
@@ -175,6 +177,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 (pcd->expected_size != chunk_size)
+		return -1;
+	*pcd->p = chunk_start;
+	return 0;
+}
+
 int pair_chunk(struct chunkfile *cf,
 	       uint32_t chunk_id,
 	       const unsigned char **p,
@@ -184,6 +197,15 @@ 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 expected_size)
+{
+	struct pair_chunk_data pcd = { .p = p, .expected_size = expected_size };
+	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..92c529d7ab 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,15 @@ int pair_chunk(struct chunkfile *cf,
 	       const unsigned char **p,
 	       size_t *size);
 
+/*
+ * Similar to 'pair_chunk', but used for callers who have an expected
+ * size for the given 'chunk_id' in advance.
+ */
+int pair_chunk_expect(struct chunkfile *cf,
+		      uint32_t chunk_id,
+		      const unsigned char **p,
+		      size_t expected_size);
+
 typedef int (*chunk_read_fn)(const unsigned char *chunk_start,
 			     size_t chunk_size, void *data);
 /*

-- 
2.42.0.352.gd9c5062ff7.dirty