reftable/writer: refactorings for `writer_add_record()`

Large parts of the reftable library do not conform to Git's typical code style. Refactor `writer_add_record()` such that it conforms better to it and add some documentation that explains some of its more intricate behaviour. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Apr 8, 2024 at 14:24 UTC d0dd119f72286d6d198911c1b33397a210e56846
1 file changed +27 -11
reftable/writer.c
+27 -11
@@ -209,7 +209,8 @@ static int writer_add_record(struct reftable_writer *w,
209 struct reftable_record *rec)
210 {
211 struct strbuf key = STRBUF_INIT;
212 - int err = -1;
212 + int err;
213 +
214 reftable_record_key(rec, &key);
215 if (strbuf_cmp(&w->last_key, &key) >= 0) {
216 err = REFTABLE_API_ERROR;
@@ -218,27 +219,42 @@ static int writer_add_record(struct reftable_writer *w,
219
220 strbuf_reset(&w->last_key);
221 strbuf_addbuf(&w->last_key, &key);
221 - if (!w->block_writer) {
222 + if (!w->block_writer)
223 writer_reinit_block_writer(w, reftable_record_type(rec));
223 - }
224
225 - assert(block_writer_type(w->block_writer) == reftable_record_type(rec));
225 + if (block_writer_type(w->block_writer) != reftable_record_type(rec))
226 + BUG("record of type %d added to writer of type %d",
227 + reftable_record_type(rec), block_writer_type(w->block_writer));
228
227 - if (block_writer_add(w->block_writer, rec) == 0) {
229 + /*
230 + * Try to add the record to the writer. If this succeeds then we're
231 + * done. Otherwise the block writer may have hit the block size limit
232 + * and needs to be flushed.
233 + */
234 + if (!block_writer_add(w->block_writer, rec)) {
235 err = 0;
236 goto done;
237 }
238
239 + /*
240 + * The current block is full, so we need to flush and reinitialize the
241 + * writer to start writing the next block.
242 + */
243 err = writer_flush_block(w);
233 - if (err < 0) {
244 + if (err < 0)
245 goto done;
235 - }
236 -
246 writer_reinit_block_writer(w, reftable_record_type(rec));
247 +
248 + /*
249 + * Try to add the record to the writer again. If this still fails then
250 + * the record does not fit into the block size.
251 + *
252 + * TODO: it would be great to have `block_writer_add()` return proper
253 + * error codes so that we don't have to second-guess the failure
254 + * mode here.
255 + */
256 err = block_writer_add(w->block_writer, rec);
239 - if (err == -1) {
240 - /* we are writing into memory, so an error can only mean it
241 - * doesn't fit. */
257 + if (err) {
258 err = REFTABLE_ENTRY_TOO_BIG_ERROR;
259 goto done;
260 }