reftable/reader: add comments to `table_iter_next()`

While working on the optimizations in the preceding patches I stumbled
upon `table_iter_next()` multiple times. It is quite easy to miss the
fact that we don't call `table_iter_next_in_block()` twice, but that the
second call is in fact `table_iter_next_block()`.

Add comments to explain what exactly is going on here to make things
more obvious. While at it, touch up the code to conform to our code
style better.

Note that one of the refactorings merges two conditional blocks into
one. Before, we had the following code:

```
err = table_iter_next_block(&next, ti);
if (err != 0) {
	ti->is_finished = 1;
}
table_iter_block_done(ti);
if (err != 0) {
	return err;
}
```

As `table_iter_block_done()` does not care about `is_finished`, the
conditional blocks can be merged into one block:

```
err = table_iter_next_block(&next, ti);
table_iter_block_done(ti);
if (err != 0) {
	ti->is_finished = 1;
	return err;
}
```

This is both easier to reason about and more performant because we have
one branch less.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/reftable/reader.c b/reftable/reader.c
index 64dc366fb15935c55c5377ec9d4f1b74640cc3f6..add7d57f0b2aadbab72121066f5037a9d4aec1b5 100644 (file)
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -357,24 +357,32 @@ static int table_iter_next(struct table_iter *ti, struct reftable_record *rec)
 
        while (1) {
                struct table_iter next = TABLE_ITER_INIT;
-               int err = 0;
-               if (ti->is_finished) {
+               int err;
+
+               if (ti->is_finished)
                        return 1;
-               }
 
+               /*
+                * Check whether the current block still has more records. If
+                * so, return it. If the iterator returns positive then the
+                * current block has been exhausted.
+                */
                err = table_iter_next_in_block(ti, rec);
-               if (err <= 0) {
+               if (err <= 0)
                        return err;
-               }
 
+               /*
+                * Otherwise, we need to continue to the next block in the
+                * table and retry. If there are no more blocks then the
+                * iterator is drained.
+                */
                err = table_iter_next_block(&next, ti);
-               if (err != 0) {
-                       ti->is_finished = 1;
-               }
                table_iter_block_done(ti);
-               if (err != 0) {
+               if (err) {
+                       ti->is_finished = 1;
                        return err;
                }
+
                table_iter_copy_from(ti, &next);
                block_iter_close(&next.bi);
        }