reftable/table: move reading block into block reader
The logic to read blocks from a reftable is scattered across both the table and the block subsystems. Besides causing somewhat fuzzy responsibilities, it also means that we have to awkwardly pass around the ownership of blocks between the subsystems. Refactor the code so that we stop passing the block when initializing a reader, but instead by passing in the block source plus the offset at which we're supposed to read a block. Like this, the ownership of the block itself doesn't need to get handed over as the block reader is the one owning the block right from the start. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Apr 7, 2025 at 15:16 UTC
fd888311fbc95b0cbb3c9e580dc6f7277bb7bf7f
4 files changed
+107
-129
reftable/block.c
+56
-31
@@ -209,31 +209,57 @@ int block_writer_finish(struct block_writer *w)
209
return w->next;
210
}
211
212
-int block_reader_init(struct block_reader *br, struct reftable_block *block,
213
- uint32_t header_off, uint32_t table_block_size,
214
- uint32_t hash_size)
212
+static int read_block(struct reftable_block_source *source,
213
+ struct reftable_block *dest, uint64_t off,
214
+ uint32_t sz)
215
{
216
+ size_t size = block_source_size(source);
217
+ block_source_return_block(dest);
218
+ if (off >= size)
219
+ return 0;
220
+ if (off + sz > size)
221
+ sz = size - off;
222
+ return block_source_read_block(source, dest, off, sz);
223
+}
224
+
225
+int block_reader_init(struct block_reader *br,
226
+ struct reftable_block_source *source,
227
+ uint32_t offset, uint32_t header_size,
228
+ uint32_t table_block_size, uint32_t hash_size)
229
+{
230
+ uint32_t guess_block_size = table_block_size ?
231
+ table_block_size : DEFAULT_BLOCK_SIZE;
232
uint32_t full_block_size = table_block_size;
217
- uint8_t typ = block->data[header_off];
218
- uint32_t sz = reftable_get_be24(block->data + header_off + 1);
233
uint16_t restart_count;
234
uint32_t restart_off;
235
+ uint32_t block_size;
236
+ uint8_t block_type;
237
int err;
238
223
- block_source_return_block(&br->block);
239
+ err = read_block(source, &br->block, offset, guess_block_size);
240
+ if (err < 0)
241
+ goto done;
242
225
- if (!reftable_is_block_type(typ)) {
226
- err = REFTABLE_FORMAT_ERROR;
243
+ block_type = br->block.data[header_size];
244
+ if (!reftable_is_block_type(block_type)) {
245
+ err = REFTABLE_FORMAT_ERROR;
246
goto done;
247
}
248
230
- if (typ == BLOCK_TYPE_LOG) {
231
- uint32_t block_header_skip = 4 + header_off;
232
- uLong dst_len = sz - block_header_skip;
233
- uLong src_len = block->len - block_header_skip;
249
+ block_size = reftable_get_be24(br->block.data + header_size + 1);
250
+ if (block_size > guess_block_size) {
251
+ err = read_block(source, &br->block, offset, block_size);
252
+ if (err < 0)
253
+ goto done;
254
+ }
255
+
256
+ if (block_type == BLOCK_TYPE_LOG) {
257
+ uint32_t block_header_skip = 4 + header_size;
258
+ uLong dst_len = block_size - block_header_skip;
259
+ uLong src_len = br->block.len - block_header_skip;
260
261
/* Log blocks specify the *uncompressed* size in their header. */
236
- REFTABLE_ALLOC_GROW_OR_NULL(br->uncompressed_data, sz,
262
+ REFTABLE_ALLOC_GROW_OR_NULL(br->uncompressed_data, block_size,
263
br->uncompressed_cap);
264
if (!br->uncompressed_data) {
265
err = REFTABLE_OUT_OF_MEMORY_ERROR;
@@ -241,7 +267,7 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
267
}
268
269
/* Copy over the block header verbatim. It's not compressed. */
244
- memcpy(br->uncompressed_data, block->data, block_header_skip);
270
+ memcpy(br->uncompressed_data, br->block.data, block_header_skip);
271
272
if (!br->zstream) {
273
REFTABLE_CALLOC_ARRAY(br->zstream, 1);
@@ -259,7 +285,7 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
285
goto done;
286
}
287
262
- br->zstream->next_in = block->data + block_header_skip;
288
+ br->zstream->next_in = br->block.data + block_header_skip;
289
br->zstream->avail_in = src_len;
290
br->zstream->next_out = br->uncompressed_data + block_header_skip;
291
br->zstream->avail_out = dst_len;
@@ -278,43 +304,41 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
304
}
305
err = 0;
306
281
- if (br->zstream->total_out + block_header_skip != sz) {
307
+ if (br->zstream->total_out + block_header_skip != block_size) {
308
err = REFTABLE_FORMAT_ERROR;
309
goto done;
310
}
311
312
/* We're done with the input data. */
287
- block_source_return_block(block);
288
- block->data = br->uncompressed_data;
289
- block->len = sz;
313
+ block_source_return_block(&br->block);
314
+ br->block.data = br->uncompressed_data;
315
+ br->block.len = block_size;
316
full_block_size = src_len + block_header_skip - br->zstream->avail_in;
317
} else if (full_block_size == 0) {
292
- full_block_size = sz;
293
- } else if (sz < full_block_size && sz < block->len &&
294
- block->data[sz] != 0) {
318
+ full_block_size = block_size;
319
+ } else if (block_size < full_block_size && block_size < br->block.len &&
320
+ br->block.data[block_size] != 0) {
321
/* If the block is smaller than the full block size, it is
322
padded (data followed by '\0') or the next block is
323
unaligned. */
298
- full_block_size = sz;
324
+ full_block_size = block_size;
325
}
326
301
- restart_count = reftable_get_be16(block->data + sz - 2);
302
- restart_off = sz - 2 - 3 * restart_count;
303
-
304
- /* transfer ownership. */
305
- br->block = *block;
306
- block->data = NULL;
307
- block->len = 0;
327
+ restart_count = reftable_get_be16(br->block.data + block_size - 2);
328
+ restart_off = block_size - 2 - 3 * restart_count;
329
330
+ br->block_type = block_type;
331
br->hash_size = hash_size;
332
br->restart_off = restart_off;
333
br->full_block_size = full_block_size;
312
- br->header_off = header_off;
334
+ br->header_off = header_size;
335
br->restart_count = restart_count;
336
337
err = 0;
338
339
done:
340
+ if (err < 0)
341
+ block_reader_release(br);
342
return err;
343
}
344
@@ -324,6 +348,7 @@ void block_reader_release(struct block_reader *br)
348
reftable_free(br->zstream);
349
reftable_free(br->uncompressed_data);
350
block_source_return_block(&br->block);
351
+ memset(br, 0, sizeof(*br));
352
}
353
354
uint8_t block_reader_type(const struct block_reader *r)
reftable/block.h
+5
-3
@@ -89,12 +89,14 @@ struct block_reader {
89
/* size of the data in the file. For log blocks, this is the compressed
90
* size. */
91
uint32_t full_block_size;
92
+ uint8_t block_type;
93
};
94
95
/* initializes a block reader. */
95
-int block_reader_init(struct block_reader *br, struct reftable_block *bl,
96
- uint32_t header_off, uint32_t table_block_size,
97
- uint32_t hash_size);
96
+int block_reader_init(struct block_reader *br,
97
+ struct reftable_block_source *source,
98
+ uint32_t offset, uint32_t header_size,
99
+ uint32_t table_block_size, uint32_t hash_size);
100
101
void block_reader_release(struct block_reader *br);
102
reftable/table.c
+6
-59
@@ -30,23 +30,6 @@ table_offsets_for(struct reftable_table *t, uint8_t typ)
30
abort();
31
}
32
33
-static int table_get_block(struct reftable_table *t,
34
- struct reftable_block *dest, uint64_t off,
35
- uint32_t sz)
36
-{
37
- ssize_t bytes_read;
38
- if (off >= t->size)
39
- return 0;
40
- if (off + sz > t->size)
41
- sz = t->size - off;
42
-
43
- bytes_read = block_source_read_block(&t->source, dest, off, sz);
44
- if (bytes_read < 0)
45
- return (int)bytes_read;
46
-
47
- return 0;
48
-}
49
-
33
enum reftable_hash reftable_table_hash_id(struct reftable_table *t)
34
{
35
return t->hash_id;
@@ -180,64 +163,28 @@ static void table_iter_block_done(struct table_iter *ti)
163
block_iter_reset(&ti->bi);
164
}
165
183
-static int32_t extract_block_size(uint8_t *data, uint8_t *typ, uint64_t off,
184
- int version)
185
-{
186
- int32_t result = 0;
187
-
188
- if (off == 0) {
189
- data += header_size(version);
190
- }
191
-
192
- *typ = data[0];
193
- if (reftable_is_block_type(*typ)) {
194
- result = reftable_get_be24(data + 1);
195
- }
196
- return result;
197
-}
198
-
166
int table_init_block_reader(struct reftable_table *t, struct block_reader *br,
167
uint64_t next_off, uint8_t want_typ)
168
{
202
- int32_t guess_block_size = t->block_size ? t->block_size :
203
- DEFAULT_BLOCK_SIZE;
204
- struct reftable_block block = { NULL };
205
- uint8_t block_typ = 0;
206
- int err = 0;
169
uint32_t header_off = next_off ? 0 : header_size(t->version);
208
- int32_t block_size = 0;
170
+ int err;
171
172
if (next_off >= t->size)
173
return 1;
174
213
- err = table_get_block(t, &block, next_off, guess_block_size);
175
+ err = block_reader_init(br, &t->source, next_off, header_off,
176
+ t->block_size, hash_size(t->hash_id));
177
if (err < 0)
178
goto done;
179
217
- block_size = extract_block_size(block.data, &block_typ, next_off,
218
- t->version);
219
- if (block_size < 0) {
220
- err = block_size;
221
- goto done;
222
- }
223
- if (want_typ != BLOCK_TYPE_ANY && block_typ != want_typ) {
180
+ if (want_typ != BLOCK_TYPE_ANY && br->block_type != want_typ) {
181
err = 1;
182
goto done;
183
}
184
228
- if (block_size > guess_block_size) {
229
- block_source_return_block(&block);
230
- err = table_get_block(t, &block, next_off, block_size);
231
- if (err < 0) {
232
- goto done;
233
- }
234
- }
235
-
236
- err = block_reader_init(br, &block, header_off, t->block_size,
237
- hash_size(t->hash_id));
185
done:
239
- block_source_return_block(&block);
240
-
186
+ if (err)
187
+ block_reader_release(br);
188
return err;
189
}
190
t/unit-tests/t-reftable-block.c
+40
-36
@@ -19,7 +19,7 @@ static void t_ref_block_read_write(void)
19
struct reftable_record recs[30];
20
const size_t N = ARRAY_SIZE(recs);
21
const size_t block_size = 1024;
22
- struct reftable_block block = { 0 };
22
+ struct reftable_block_source source = { 0 };
23
struct block_writer bw = {
24
.last_key = REFTABLE_BUF_INIT,
25
};
@@ -30,13 +30,14 @@ static void t_ref_block_read_write(void)
30
int ret;
31
struct block_reader br = { 0 };
32
struct block_iter it = BLOCK_ITER_INIT;
33
- struct reftable_buf want = REFTABLE_BUF_INIT, buf = REFTABLE_BUF_INIT;
33
+ struct reftable_buf want = REFTABLE_BUF_INIT;
34
+ struct reftable_buf block = REFTABLE_BUF_INIT;
35
35
- REFTABLE_CALLOC_ARRAY(block.data, block_size);
36
- check(block.data != NULL);
36
+ REFTABLE_CALLOC_ARRAY(block.buf, block_size);
37
+ check(block.buf != NULL);
38
block.len = block_size;
38
- block_source_from_buf(&block.source ,&buf);
39
- ret = block_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size,
39
+
40
+ ret = block_writer_init(&bw, BLOCK_TYPE_REF, (uint8_t *) block.buf, block_size,
41
header_off, hash_size(REFTABLE_HASH_SHA1));
42
check(!ret);
43
@@ -62,7 +63,8 @@ static void t_ref_block_read_write(void)
63
64
block_writer_release(&bw);
65
65
- block_reader_init(&br, &block, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
66
+ block_source_from_buf(&source ,&block);
67
+ block_reader_init(&br, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
68
69
block_iter_seek_start(&it, &br);
70
@@ -100,9 +102,8 @@ static void t_ref_block_read_write(void)
102
block_reader_release(&br);
103
block_iter_close(&it);
104
reftable_record_release(&rec);
103
- block_source_return_block(&br.block);
105
reftable_buf_release(&want);
105
- reftable_buf_release(&buf);
106
+ reftable_buf_release(&block);
107
for (i = 0; i < N; i++)
108
reftable_record_release(&recs[i]);
109
}
@@ -113,7 +114,7 @@ static void t_log_block_read_write(void)
114
struct reftable_record recs[30];
115
const size_t N = ARRAY_SIZE(recs);
116
const size_t block_size = 2048;
116
- struct reftable_block block = { 0 };
117
+ struct reftable_block_source source = { 0 };
118
struct block_writer bw = {
119
.last_key = REFTABLE_BUF_INIT,
120
};
@@ -124,13 +125,14 @@ static void t_log_block_read_write(void)
125
int ret;
126
struct block_reader br = { 0 };
127
struct block_iter it = BLOCK_ITER_INIT;
127
- struct reftable_buf want = REFTABLE_BUF_INIT, buf = REFTABLE_BUF_INIT;
128
+ struct reftable_buf want = REFTABLE_BUF_INIT;
129
+ struct reftable_buf block = REFTABLE_BUF_INIT;
130
129
- REFTABLE_CALLOC_ARRAY(block.data, block_size);
130
- check(block.data != NULL);
131
+ REFTABLE_CALLOC_ARRAY(block.buf, block_size);
132
+ check(block.buf != NULL);
133
block.len = block_size;
132
- block_source_from_buf(&block.source ,&buf);
133
- ret = block_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,
134
+
135
+ ret = block_writer_init(&bw, BLOCK_TYPE_LOG, (uint8_t *) block.buf, block_size,
136
header_off, hash_size(REFTABLE_HASH_SHA1));
137
check(!ret);
138
@@ -151,7 +153,8 @@ static void t_log_block_read_write(void)
153
154
block_writer_release(&bw);
155
154
- block_reader_init(&br, &block, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
156
+ block_source_from_buf(&source, &block);
157
+ block_reader_init(&br, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
158
159
block_iter_seek_start(&it, &br);
160
@@ -190,9 +193,8 @@ static void t_log_block_read_write(void)
193
block_reader_release(&br);
194
block_iter_close(&it);
195
reftable_record_release(&rec);
193
- block_source_return_block(&br.block);
196
reftable_buf_release(&want);
195
- reftable_buf_release(&buf);
197
+ reftable_buf_release(&block);
198
for (i = 0; i < N; i++)
199
reftable_record_release(&recs[i]);
200
}
@@ -203,7 +205,7 @@ static void t_obj_block_read_write(void)
205
struct reftable_record recs[30];
206
const size_t N = ARRAY_SIZE(recs);
207
const size_t block_size = 1024;
206
- struct reftable_block block = { 0 };
208
+ struct reftable_block_source source = { 0 };
209
struct block_writer bw = {
210
.last_key = REFTABLE_BUF_INIT,
211
};
@@ -214,13 +216,14 @@ static void t_obj_block_read_write(void)
216
int ret;
217
struct block_reader br = { 0 };
218
struct block_iter it = BLOCK_ITER_INIT;
217
- struct reftable_buf want = REFTABLE_BUF_INIT, buf = REFTABLE_BUF_INIT;
219
+ struct reftable_buf want = REFTABLE_BUF_INIT;
220
+ struct reftable_buf block = REFTABLE_BUF_INIT;
221
219
- REFTABLE_CALLOC_ARRAY(block.data, block_size);
220
- check(block.data != NULL);
222
+ REFTABLE_CALLOC_ARRAY(block.buf, block_size);
223
+ check(block.buf != NULL);
224
block.len = block_size;
222
- block_source_from_buf(&block.source, &buf);
223
- ret = block_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,
225
+
226
+ ret = block_writer_init(&bw, BLOCK_TYPE_OBJ, (uint8_t *) block.buf, block_size,
227
header_off, hash_size(REFTABLE_HASH_SHA1));
228
check(!ret);
229
@@ -243,7 +246,8 @@ static void t_obj_block_read_write(void)
246
247
block_writer_release(&bw);
248
246
- block_reader_init(&br, &block, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
249
+ block_source_from_buf(&source, &block);
250
+ block_reader_init(&br, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
251
252
block_iter_seek_start(&it, &br);
253
@@ -273,9 +277,8 @@ static void t_obj_block_read_write(void)
277
block_reader_release(&br);
278
block_iter_close(&it);
279
reftable_record_release(&rec);
276
- block_source_return_block(&br.block);
280
reftable_buf_release(&want);
278
- reftable_buf_release(&buf);
281
+ reftable_buf_release(&block);
282
for (i = 0; i < N; i++)
283
reftable_record_release(&recs[i]);
284
}
@@ -286,7 +289,7 @@ static void t_index_block_read_write(void)
289
struct reftable_record recs[30];
290
const size_t N = ARRAY_SIZE(recs);
291
const size_t block_size = 1024;
289
- struct reftable_block block = { 0 };
292
+ struct reftable_block_source source = { 0 };
293
struct block_writer bw = {
294
.last_key = REFTABLE_BUF_INIT,
295
};
@@ -298,13 +301,14 @@ static void t_index_block_read_write(void)
301
int ret;
302
struct block_reader br = { 0 };
303
struct block_iter it = BLOCK_ITER_INIT;
301
- struct reftable_buf want = REFTABLE_BUF_INIT, buf = REFTABLE_BUF_INIT;
304
+ struct reftable_buf want = REFTABLE_BUF_INIT;
305
+ struct reftable_buf block = REFTABLE_BUF_INIT;
306
303
- REFTABLE_CALLOC_ARRAY(block.data, block_size);
304
- check(block.data != NULL);
307
+ REFTABLE_CALLOC_ARRAY(block.buf, block_size);
308
+ check(block.buf != NULL);
309
block.len = block_size;
306
- block_source_from_buf(&block.source, &buf);
307
- ret = block_writer_init(&bw, BLOCK_TYPE_INDEX, block.data, block_size,
310
+
311
+ ret = block_writer_init(&bw, BLOCK_TYPE_INDEX, (uint8_t *) block.buf, block_size,
312
header_off, hash_size(REFTABLE_HASH_SHA1));
313
check(!ret);
314
@@ -327,7 +331,8 @@ static void t_index_block_read_write(void)
331
332
block_writer_release(&bw);
333
330
- block_reader_init(&br, &block, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
334
+ block_source_from_buf(&source, &block);
335
+ block_reader_init(&br, &source, 0, header_off, block_size, REFTABLE_HASH_SIZE_SHA1);
336
337
block_iter_seek_start(&it, &br);
338
@@ -365,9 +370,8 @@ static void t_index_block_read_write(void)
370
block_reader_release(&br);
371
block_iter_close(&it);
372
reftable_record_release(&rec);
368
- block_source_return_block(&br.block);
373
reftable_buf_release(&want);
370
- reftable_buf_release(&buf);
374
+ reftable_buf_release(&block);
375
for (i = 0; i < N; i++)
376
reftable_record_release(&recs[i]);
377
}