midx: use chunk-format read API

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Feb 18, 2021 at 14:07 UTC 6ab3b8b8b8dc94c8e4caefb1d368cc704e70d38b
2 files changed +29 -50
midx.c
+26 -47
@@ -28,7 +28,6 @@
28 #define MIDX_CHUNKID_OIDLOOKUP 0x4f49444c /* "OIDL" */
29 #define MIDX_CHUNKID_OBJECTOFFSETS 0x4f4f4646 /* "OOFF" */
30 #define MIDX_CHUNKID_LARGEOFFSETS 0x4c4f4646 /* "LOFF" */
31 -#define MIDX_CHUNKLOOKUP_WIDTH (sizeof(uint32_t) + sizeof(uint64_t))
31 #define MIDX_CHUNK_FANOUT_SIZE (sizeof(uint32_t) * 256)
32 #define MIDX_CHUNK_OFFSET_WIDTH (2 * sizeof(uint32_t))
33 #define MIDX_CHUNK_LARGE_OFFSET_WIDTH (sizeof(uint64_t))
@@ -53,6 +52,19 @@ static char *get_midx_filename(const char *object_dir)
52 return xstrfmt("%s/pack/multi-pack-index", object_dir);
53 }
54
55 +static int midx_read_oid_fanout(const unsigned char *chunk_start,
56 + size_t chunk_size, void *data)
57 +{
58 + struct multi_pack_index *m = data;
59 + m->chunk_oid_fanout = (uint32_t *)chunk_start;
60 +
61 + if (chunk_size != 4 * 256) {
62 + error(_("multi-pack-index OID fanout is of the wrong size"));
63 + return 1;
64 + }
65 + return 0;
66 +}
67 +
68 struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local)
69 {
70 struct multi_pack_index *m = NULL;
@@ -64,6 +76,7 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
76 char *midx_name = get_midx_filename(object_dir);
77 uint32_t i;
78 const char *cur_pack_name;
79 + struct chunkfile *cf = NULL;
80
81 fd = git_open(midx_name);
82
@@ -113,58 +126,23 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
126
127 m->num_packs = get_be32(m->data + MIDX_BYTE_NUM_PACKS);
128
116 - for (i = 0; i < m->num_chunks; i++) {
117 - uint32_t chunk_id = get_be32(m->data + MIDX_HEADER_SIZE +
118 - MIDX_CHUNKLOOKUP_WIDTH * i);
119 - uint64_t chunk_offset = get_be64(m->data + MIDX_HEADER_SIZE + 4 +
120 - MIDX_CHUNKLOOKUP_WIDTH * i);
121 -
122 - if (chunk_offset >= m->data_len)
123 - die(_("invalid chunk offset (too large)"));
124 -
125 - switch (chunk_id) {
126 - case MIDX_CHUNKID_PACKNAMES:
127 - m->chunk_pack_names = m->data + chunk_offset;
128 - break;
129 -
130 - case MIDX_CHUNKID_OIDFANOUT:
131 - m->chunk_oid_fanout = (uint32_t *)(m->data + chunk_offset);
132 - break;
133 -
134 - case MIDX_CHUNKID_OIDLOOKUP:
135 - m->chunk_oid_lookup = m->data + chunk_offset;
136 - break;
137 -
138 - case MIDX_CHUNKID_OBJECTOFFSETS:
139 - m->chunk_object_offsets = m->data + chunk_offset;
140 - break;
141 -
142 - case MIDX_CHUNKID_LARGEOFFSETS:
143 - m->chunk_large_offsets = m->data + chunk_offset;
144 - break;
145 -
146 - case 0:
147 - die(_("terminating multi-pack-index chunk id appears earlier than expected"));
148 - break;
149 -
150 - default:
151 - /*
152 - * Do nothing on unrecognized chunks, allowing future
153 - * extensions to add optional chunks.
154 - */
155 - break;
156 - }
157 - }
129 + cf = init_chunkfile(NULL);
130
159 - if (!m->chunk_pack_names)
131 + if (read_table_of_contents(cf, m->data, midx_size,
132 + MIDX_HEADER_SIZE, m->num_chunks))
133 + goto cleanup_fail;
134 +
135 + if (pair_chunk(cf, MIDX_CHUNKID_PACKNAMES, &m->chunk_pack_names) == CHUNK_NOT_FOUND)
136 die(_("multi-pack-index missing required pack-name chunk"));
161 - if (!m->chunk_oid_fanout)
137 + if (read_chunk(cf, MIDX_CHUNKID_OIDFANOUT, midx_read_oid_fanout, m) == CHUNK_NOT_FOUND)
138 die(_("multi-pack-index missing required OID fanout chunk"));
163 - if (!m->chunk_oid_lookup)
139 + if (pair_chunk(cf, MIDX_CHUNKID_OIDLOOKUP, &m->chunk_oid_lookup) == CHUNK_NOT_FOUND)
140 die(_("multi-pack-index missing required OID lookup chunk"));
165 - if (!m->chunk_object_offsets)
141 + if (pair_chunk(cf, MIDX_CHUNKID_OBJECTOFFSETS, &m->chunk_object_offsets) == CHUNK_NOT_FOUND)
142 die(_("multi-pack-index missing required object offsets chunk"));
143
144 + pair_chunk(cf, MIDX_CHUNKID_LARGEOFFSETS, &m->chunk_large_offsets);
145 +
146 m->num_objects = ntohl(m->chunk_oid_fanout[255]);
147
148 m->pack_names = xcalloc(m->num_packs, sizeof(*m->pack_names));
@@ -190,6 +168,7 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
168 cleanup_fail:
169 free(m);
170 free(midx_name);
171 + free(cf);
172 if (midx_map)
173 munmap(midx_map, midx_size);
174 if (0 <= fd)
t/t5319-multi-pack-index.sh
+3 -3
@@ -314,12 +314,12 @@ test_expect_success 'verify bad OID version' '
314
315 test_expect_success 'verify truncated chunk count' '
316 corrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT "\01" $objdir \
317 - "missing required"
317 + "final chunk has non-zero id"
318 '
319
320 test_expect_success 'verify extended chunk count' '
321 corrupt_midx_and_verify $MIDX_BYTE_CHUNK_COUNT "\07" $objdir \
322 - "terminating multi-pack-index chunk id appears earlier than expected"
322 + "terminating chunk id appears earlier than expected"
323 '
324
325 test_expect_success 'verify missing required chunk' '
@@ -329,7 +329,7 @@ test_expect_success 'verify missing required chunk' '
329
330 test_expect_success 'verify invalid chunk offset' '
331 corrupt_midx_and_verify $MIDX_BYTE_CHUNK_OFFSET "\01" $objdir \
332 - "invalid chunk offset (too large)"
332 + "improper chunk offset(s)"
333 '
334
335 test_expect_success 'verify packnames out of order' '