git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 7/9] reftable/merged: refactor seeking of records

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 1, 2024, 07:33 UTC
Message-ID
<80cf2fd272577b13f4fd23f009d22294629f5f3b.1706772591.git.ps@pks.im>
In-Reply-To
<cover.1706772591.git.ps@pks.im>

The code to seek reftable records in the merged table code is quite hard to read and does not conform to our coding style in multiple ways:

  - We have multiple exit paths where we release resources even though
    that is not really necessary.
  - We use a scoped error variable `e` which is hard to reason about.
    This variable is not required at all.
  - We allocate memory in the variable declarations, which is easy to
    miss.

Refactor the function so that it becomes more maintainable in the future.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/merged.c | 54 ++++++++++++++++++-----------------------------
 1 file changed, 21 insertions(+), 33 deletions(-)
diff --git a/reftable/merged.c b/reftable/merged.c
index e2c6253324..0abcda26e8 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -238,50 +238,38 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
 				    struct reftable_iterator *it,
 				    struct reftable_record *rec)
 {
-	struct reftable_iterator *iters = reftable_calloc(
-		mt->stack_len, sizeof(*iters));
 	struct merged_iter merged = {
-		.stack = iters,
 		.typ = reftable_record_type(rec),
 		.hash_id = mt->hash_id,
 		.suppress_deletions = mt->suppress_deletions,
 		.key = STRBUF_INIT,
 		.entry_key = STRBUF_INIT,
 	};
-	int n = 0;
-	int err = 0;
-	int i = 0;
-	for (i = 0; i < mt->stack_len && err == 0; i++) {
-		int e = reftable_table_seek_record(&mt->stack[i], &iters[n],
-						   rec);
-		if (e < 0) {
-			err = e;
-		}
-		if (e == 0) {
-			n++;
-		}
-	}
-	if (err < 0) {
-		int i = 0;
-		for (i = 0; i < n; i++) {
-			reftable_iterator_destroy(&iters[i]);
-		}
-		reftable_free(iters);
-		return err;
+	struct merged_iter *p;
+	int err;
+
+	REFTABLE_CALLOC_ARRAY(merged.stack, mt->stack_len);
+	for (size_t i = 0; i < mt->stack_len; i++) {
+		err = reftable_table_seek_record(&mt->stack[i],
+						 &merged.stack[merged.stack_len], rec);
+		if (err < 0)
+			goto out;
+		if (!err)
+			merged.stack_len++;
 	}
 
-	merged.stack_len = n;
 	err = merged_iter_init(&merged);
-	if (err < 0) {
+	if (err < 0)
+		goto out;
+
+	p = reftable_malloc(sizeof(struct merged_iter));
+	*p = merged;
+	iterator_from_merged_iter(it, p);
+
+out:
+	if (err < 0)
 		merged_iter_close(&merged);
-		return err;
-	} else {
-		struct merged_iter *p =
-			reftable_malloc(sizeof(struct merged_iter));
-		*p = merged;
-		iterator_from_merged_iter(it, p);
-	}
-	return 0;
+	return err;
 }
 
 int reftable_merged_table_seek_ref(struct reftable_merged_table *mt,
-- 
2.43.GIT
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 27 of 40 in “reftable: code style improvements”
  1. 0/9 reftable: code style improvementsPatrick Steinhardt, Jan 31, 2024
  2. 1/9 reftable: introduce macros to grow arraysPatrick Steinhardt, Jan 31, 2024
  3. Eric SunshineJan 31, 2024
  4. Junio C HamanoJan 31, 2024
  5. Patrick SteinhardtFeb 1, 2024
  6. Junio C HamanoFeb 1, 2024
  7. 2/9 reftable: introduce macros to allocate arraysPatrick Steinhardt, Jan 31, 2024
  8. 3/9 reftable/stack: fix parameter validation when compacting rangePatrick Steinhardt, Jan 31, 2024
  9. 4/9 reftable/stack: index segments with `size_t`Patrick Steinhardt, Jan 31, 2024
  10. 5/9 reftable/stack: use `size_t` to track stack slices during compactionPatrick Steinhardt, Jan 31, 2024
  11. 6/9 reftable/stack: use `size_t` to track stack lengthPatrick Steinhardt, Jan 31, 2024
  12. 7/9 reftable/merged: refactor seeking of recordsPatrick Steinhardt, Jan 31, 2024
  13. Eric SunshineJan 31, 2024
  14. 8/9 reftable/merged: refactor initialization of iteratorsPatrick Steinhardt, Jan 31, 2024
  15. 9/9 reftable/record: improve semantics when initializing recordsPatrick Steinhardt, Jan 31, 2024
  16. 0/9 reftable: code style improvementsPatrick Steinhardt, Feb 1, 2024
  17. 1/9 reftable: introduce macros to grow arraysPatrick Steinhardt, Feb 1, 2024
  18. 2/9 reftable: introduce macros to allocate arraysPatrick Steinhardt, Feb 1, 2024
  19. Karthik NayakFeb 5, 2024
  20. Patrick SteinhardtFeb 6, 2024
  21. 3/9 reftable/stack: fix parameter validation when compacting rangePatrick Steinhardt, Feb 1, 2024
  22. Toon ClaesFeb 1, 2024
  23. Patrick SteinhardtFeb 2, 2024
  24. 4/9 reftable/stack: index segments with `size_t`Patrick Steinhardt, Feb 1, 2024
  25. 5/9 reftable/stack: use `size_t` to track stack slices during compactionPatrick Steinhardt, Feb 1, 2024
  26. 6/9 reftable/stack: use `size_t` to track stack lengthPatrick Steinhardt, Feb 1, 2024
  27. 7/9 reftable/merged: refactor seeking of recordsPatrick Steinhardt, Feb 1, 2024
  28. 8/9 reftable/merged: refactor initialization of iteratorsPatrick Steinhardt, Feb 1, 2024
  29. 9/9 reftable/record: improve semantics when initializing recordsPatrick Steinhardt, Feb 1, 2024
  30. 0/9 reftable: code style improvementsPatrick Steinhardt, Feb 6, 2024
  31. 1/9 reftable: introduce macros to grow arraysPatrick Steinhardt, Feb 6, 2024
  32. 2/9 reftable: introduce macros to allocate arraysPatrick Steinhardt, Feb 6, 2024
  33. 3/9 reftable/stack: fix parameter validation when compacting rangePatrick Steinhardt, Feb 6, 2024
  34. 4/9 reftable/stack: index segments with `size_t`Patrick Steinhardt, Feb 6, 2024
  35. 5/9 reftable/stack: use `size_t` to track stack slices during compactionPatrick Steinhardt, Feb 6, 2024
  36. 6/9 reftable/stack: use `size_t` to track stack lengthPatrick Steinhardt, Feb 6, 2024
  37. 7/9 reftable/merged: refactor seeking of recordsPatrick Steinhardt, Feb 6, 2024
  38. 8/9 reftable/merged: refactor initialization of iteratorsPatrick Steinhardt, Feb 6, 2024
  39. 9/9 reftable/record: improve semantics when initializing recordsPatrick Steinhardt, Feb 6, 2024
  40. Karthik NayakFeb 6, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.