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

[PATCH 10/10] reftable: handle trivial `reftable_buf` errors

From
Patrick Steinhardt <ps@pks.im>
Date
Oct 11, 2024, 06:54 UTC
Message-ID
<8406948ae747844c3dfb1b5454218d116945897a.1728629612.git.ps@pks.im>
In-Reply-To
<cover.1728629612.git.ps@pks.im>

Convert the reftable library such that we handle failures with the new `reftable_buf` interfaces.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/block.c  |  9 +++++++--
 reftable/iter.c   |  5 ++++-
 reftable/reader.c |  5 ++++-
 reftable/record.c | 32 +++++++++++++++++++++++--------
 reftable/stack.c  | 49 +++++++++++++++++++++++++++++++----------------
 reftable/writer.c | 48 +++++++++++++++++++++++++++++++++-------------
 6 files changed, 107 insertions(+), 41 deletions(-)
diff --git a/reftable/block.c b/reftable/block.c
index 697b8b41531..f5b432566a6 100644
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -40,7 +40,9 @@ int footer_size(int version)
 static int block_writer_register_restart(struct block_writer *w, int n,
 					 int is_restart, struct reftable_buf *key)
 {
-	int rlen = w->restart_len;
+	int rlen, err;
+
+	rlen = w->restart_len;
 	if (rlen >= MAX_RESTARTS) {
 		is_restart = 0;
 	}
@@ -60,7 +62,10 @@ static int block_writer_register_restart(struct block_writer *w, int n,
 	w->next += n;
 
 	reftable_buf_reset(&w->last_key);
-	reftable_buf_add(&w->last_key, key->buf, key->len);
+	err = reftable_buf_add(&w->last_key, key->buf, key->len);
+	if (err < 0)
+		return err;
+
 	w->entries++;
 	return 0;
 }
diff --git a/reftable/iter.c b/reftable/iter.c
index 6c193fd31a9..86e801ca9fb 100644
--- a/reftable/iter.c
+++ b/reftable/iter.c
@@ -197,7 +197,10 @@ int indexed_table_ref_iter_new(struct indexed_table_ref_iter **dest,
 
 	*itr = empty;
 	itr->r = r;
-	reftable_buf_add(&itr->oid, oid, oid_len);
+
+	err = reftable_buf_add(&itr->oid, oid, oid_len);
+	if (err < 0)
+		goto out;
 
 	itr->offsets = offsets;
 	itr->offset_len = offset_len;
diff --git a/reftable/reader.c b/reftable/reader.c
index ab89efd9c55..90dc950b577 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -769,7 +769,10 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r,
 	}
 	*filter = empty;
 
-	reftable_buf_add(&filter->oid, oid, oid_len);
+	err = reftable_buf_add(&filter->oid, oid, oid_len);
+	if (err < 0)
+		goto out;
+
 	iterator_from_table_iter(&filter->it, ti);
 
 	iterator_from_filtering_ref_iterator(it, filter);
diff --git a/reftable/record.c b/reftable/record.c
index 672c5f909a9..fb5652ed575 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -102,7 +102,9 @@ static int decode_string(struct reftable_buf *dest, struct string_view in)
 {
 	int start_len = in.len;
 	uint64_t tsize = 0;
-	int n = get_var_int(&tsize, &in);
+	int n, err;
+
+	n = get_var_int(&tsize, &in);
 	if (n <= 0)
 		return -1;
 	string_view_consume(&in, n);
@@ -110,7 +112,10 @@ static int decode_string(struct reftable_buf *dest, struct string_view in)
 		return -1;
 
 	reftable_buf_reset(dest);
-	reftable_buf_add(dest, in.buf, tsize);
+	err = reftable_buf_add(dest, in.buf, tsize);
+	if (err < 0)
+		return err;
+
 	string_view_consume(&in, tsize);
 
 	return start_len - in.len;
@@ -189,7 +194,7 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra,
 	int start_len = in.len;
 	uint64_t prefix_len = 0;
 	uint64_t suffix_len = 0;
-	int n;
+	int err, n;
 
 	n = reftable_decode_keylen(in, &prefix_len, &suffix_len, extra);
 	if (n < 0)
@@ -200,8 +205,14 @@ int reftable_decode_key(struct reftable_buf *last_key, uint8_t *extra,
 	    prefix_len > last_key->len)
 		return -1;
 
-	reftable_buf_setlen(last_key, prefix_len);
-	reftable_buf_add(last_key, in.buf, suffix_len);
+	err = reftable_buf_setlen(last_key, prefix_len);
+	if (err < 0)
+		return err;
+
+	err = reftable_buf_add(last_key, in.buf, suffix_len);
+	if (err < 0)
+		return err;
+
 	string_view_consume(&in, suffix_len);
 
 	return start_len - in.len;
@@ -1047,9 +1058,12 @@ static int reftable_index_record_copy_from(void *rec, const void *src_rec,
 {
 	struct reftable_index_record *dst = rec;
 	const struct reftable_index_record *src = src_rec;
+	int err;
 
 	reftable_buf_reset(&dst->last_key);
-	reftable_buf_add(&dst->last_key, src->last_key.buf, src->last_key.len);
+	err = reftable_buf_add(&dst->last_key, src->last_key.buf, src->last_key.len);
+	if (err < 0)
+		return err;
 	dst->offset = src->offset;
 
 	return 0;
@@ -1090,10 +1104,12 @@ static int reftable_index_record_decode(void *rec, struct reftable_buf key,
 {
 	struct string_view start = in;
 	struct reftable_index_record *r = rec;
-	int n = 0;
+	int err, n = 0;
 
 	reftable_buf_reset(&r->last_key);
-	reftable_buf_add(&r->last_key, key.buf, key.len);
+	err = reftable_buf_add(&r->last_key, key.buf, key.len);
+	if (err < 0)
+		return err;
 
 	n = get_var_int(&r->offset, &in);
 	if (n < 0)
diff --git a/reftable/stack.c b/reftable/stack.c
index 243b10715cc..c33979536ef 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -78,8 +78,9 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,
 	*dest = NULL;
 
 	reftable_buf_reset(&list_file_name);
-	reftable_buf_addstr(&list_file_name, dir);
-	reftable_buf_addstr(&list_file_name, "/tables.list");
+	if ((err = reftable_buf_addstr(&list_file_name, dir)) < 0 ||
+	    (err = reftable_buf_addstr(&list_file_name, "/tables.list")) < 0)
+		goto out;
 
 	p->list_file = reftable_buf_detach(&list_file_name);
 	p->list_fd = -1;
@@ -747,12 +748,14 @@ int reftable_addition_commit(struct reftable_addition *add)
 		goto done;
 
 	for (i = 0; i < add->stack->merged->readers_len; i++) {
-		reftable_buf_addstr(&table_list, add->stack->readers[i]->name);
-		reftable_buf_addstr(&table_list, "\n");
+		if ((err = reftable_buf_addstr(&table_list, add->stack->readers[i]->name)) < 0 ||
+		    (err = reftable_buf_addstr(&table_list, "\n")) < 0)
+			goto done;
 	}
 	for (i = 0; i < add->new_tables_len; i++) {
-		reftable_buf_addstr(&table_list, add->new_tables[i]);
-		reftable_buf_addstr(&table_list, "\n");
+		if ((err = reftable_buf_addstr(&table_list, add->new_tables[i])) < 0 ||
+		    (err = reftable_buf_addstr(&table_list, "\n")) < 0)
+			goto done;
 	}
 
 	err = write_in_full(lock_file_fd, table_list.buf, table_list.len);
@@ -867,7 +870,10 @@ int reftable_addition_add(struct reftable_addition *add,
 	err = stack_filename(&temp_tab_file_name, add->stack, next_name.buf);
 	if (err < 0)
 		goto done;
-	reftable_buf_addstr(&temp_tab_file_name, ".temp.XXXXXX");
+
+	err = reftable_buf_addstr(&temp_tab_file_name, ".temp.XXXXXX");
+	if (err < 0)
+		goto done;
 
 	tab_file = mks_tempfile(temp_tab_file_name.buf);
 	if (!tab_file) {
@@ -914,7 +920,10 @@ int reftable_addition_add(struct reftable_addition *add,
 	err = format_name(&next_name, wr->min_update_index, wr->max_update_index);
 	if (err < 0)
 		goto done;
-	reftable_buf_addstr(&next_name, ".ref");
+
+	err = reftable_buf_addstr(&next_name, ".ref");
+	if (err < 0)
+		goto done;
 
 	err = stack_filename(&tab_file_name, add->stack, next_name.buf);
 	if (err < 0)
@@ -975,7 +984,10 @@ static int stack_compact_locked(struct reftable_stack *st,
 	err = stack_filename(&tab_file_path, st, next_name.buf);
 	if (err < 0)
 		goto done;
-	reftable_buf_addstr(&tab_file_path, ".temp.XXXXXX");
+
+	err = reftable_buf_addstr(&tab_file_path, ".temp.XXXXXX");
+	if (err < 0)
+		goto done;
 
 	tab_file = mks_tempfile(tab_file_path.buf);
 	if (!tab_file) {
@@ -1404,7 +1416,9 @@ static int stack_compact_range(struct reftable_stack *st,
 		if (err < 0)
 			goto done;
 
-		reftable_buf_addstr(&new_table_name, ".ref");
+		err = reftable_buf_addstr(&new_table_name, ".ref");
+		if (err < 0)
+			goto done;
 
 		err = stack_filename(&new_table_path, st, new_table_name.buf);
 		if (err < 0)
@@ -1423,16 +1437,19 @@ static int stack_compact_range(struct reftable_stack *st,
 	 * simply skip writing it.
 	 */
 	for (i = 0; i < first_to_replace; i++) {
-		reftable_buf_addstr(&tables_list_buf, names[i]);
-		reftable_buf_addstr(&tables_list_buf, "\n");
+		if ((err = reftable_buf_addstr(&tables_list_buf, names[i])) < 0 ||
+		    (err = reftable_buf_addstr(&tables_list_buf, "\n")) < 0)
+		      goto done;
 	}
 	if (!is_empty_table) {
-		reftable_buf_addstr(&tables_list_buf, new_table_name.buf);
-		reftable_buf_addstr(&tables_list_buf, "\n");
+		if ((err = reftable_buf_addstr(&tables_list_buf, new_table_name.buf)) < 0 ||
+		    (err = reftable_buf_addstr(&tables_list_buf, "\n")) < 0)
+			goto done;
 	}
 	for (i = last_to_replace + 1; names[i]; i++) {
-		reftable_buf_addstr(&tables_list_buf, names[i]);
-		reftable_buf_addstr(&tables_list_buf, "\n");
+		if ((err = reftable_buf_addstr(&tables_list_buf, names[i])) < 0 ||
+		    (err = reftable_buf_addstr(&tables_list_buf, "\n")) < 0)
+			goto done;
 	}
 
 	err = write_in_full(get_lock_file_fd(&tables_list_lock),
diff --git a/reftable/writer.c b/reftable/writer.c
index 377db709c85..fd136794d5a 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -217,6 +217,7 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has
 	node = tree_search(w->obj_index_tree, &want, &obj_index_tree_node_compare);
 	if (!node) {
 		struct obj_index_tree_node empty = OBJ_INDEX_TREE_NODE_INIT;
+		int err;
 
 		key = reftable_malloc(sizeof(*key));
 		if (!key)
@@ -225,7 +226,9 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has
 		*key = empty;
 
 		reftable_buf_reset(&key->hash);
-		reftable_buf_add(&key->hash, hash->buf, hash->len);
+		err = reftable_buf_add(&key->hash, hash->buf, hash->len);
+		if (err < 0)
+			return err;
 		tree_insert(&w->obj_index_tree, key,
 			    &obj_index_tree_node_compare);
 	} else {
@@ -259,7 +262,10 @@ static int writer_add_record(struct reftable_writer *w,
 	}
 
 	reftable_buf_reset(&w->last_key);
-	reftable_buf_add(&w->last_key, key.buf, key.len);
+	err = reftable_buf_add(&w->last_key, key.buf, key.len);
+	if (err < 0)
+		goto done;
+
 	if (!w->block_writer) {
 		err = writer_reinit_block_writer(w, reftable_record_type(rec));
 		if (err < 0)
@@ -334,8 +340,10 @@ int reftable_writer_add_ref(struct reftable_writer *w,
 		goto out;
 
 	if (!w->opts.skip_index_objects && reftable_ref_record_val1(ref)) {
-		reftable_buf_add(&buf, (char *)reftable_ref_record_val1(ref),
-			   hash_size(w->opts.hash_id));
+		err = reftable_buf_add(&buf, (char *)reftable_ref_record_val1(ref),
+				       hash_size(w->opts.hash_id));
+		if (err < 0)
+			goto out;
 
 		err = writer_index_hash(w, &buf);
 		if (err < 0)
@@ -344,8 +352,10 @@ int reftable_writer_add_ref(struct reftable_writer *w,
 
 	if (!w->opts.skip_index_objects && reftable_ref_record_val2(ref)) {
 		reftable_buf_reset(&buf);
-		reftable_buf_add(&buf, reftable_ref_record_val2(ref),
-			   hash_size(w->opts.hash_id));
+		err = reftable_buf_add(&buf, reftable_ref_record_val2(ref),
+				       hash_size(w->opts.hash_id));
+		if (err < 0)
+			goto out;
 
 		err = writer_index_hash(w, &buf);
 		if (err < 0)
@@ -407,17 +417,27 @@ int reftable_writer_add_log(struct reftable_writer *w,
 
 	input_log_message = log->value.update.message;
 	if (!w->opts.exact_log_message && log->value.update.message) {
-		reftable_buf_addstr(&cleaned_message, log->value.update.message);
+		err = reftable_buf_addstr(&cleaned_message, log->value.update.message);
+		if (err < 0)
+			goto done;
+
 		while (cleaned_message.len &&
-		       cleaned_message.buf[cleaned_message.len - 1] == '\n')
-			reftable_buf_setlen(&cleaned_message,
-				      cleaned_message.len - 1);
+		       cleaned_message.buf[cleaned_message.len - 1] == '\n') {
+			err = reftable_buf_setlen(&cleaned_message,
+						  cleaned_message.len - 1);
+			if (err < 0)
+				goto done;
+		}
 		if (strchr(cleaned_message.buf, '\n')) {
 			/* multiple lines not allowed. */
 			err = REFTABLE_API_ERROR;
 			goto done;
 		}
-		reftable_buf_addstr(&cleaned_message, "\n");
+
+		err = reftable_buf_addstr(&cleaned_message, "\n");
+		if (err < 0)
+			goto done;
+
 		log->value.update.message = cleaned_message.buf;
 	}
 
@@ -781,8 +801,10 @@ static int writer_flush_nonempty_block(struct reftable_writer *w)
 
 	index_record.offset = w->next;
 	reftable_buf_reset(&index_record.last_key);
-	reftable_buf_add(&index_record.last_key, w->block_writer->last_key.buf,
-		   w->block_writer->last_key.len);
+	err = reftable_buf_add(&index_record.last_key, w->block_writer->last_key.buf,
+			       w->block_writer->last_key.len);
+	if (err < 0)
+		return err;
 	w->index[w->index_len] = index_record;
 	w->index_len++;
 

-- 
2.47.0.dirty
Previous: Patrick SteinhardtNext: karthik nayak
Message 17 of 63 in “reftable: stop using `struct strbuf`”
  1. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 11, 2024
  2. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 11, 2024
  3. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 11, 2024
  4. karthik nayakOct 11, 2024
  5. Patrick SteinhardtOct 14, 2024
  6. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 11, 2024
  7. karthik nayakOct 11, 2024
  8. Patrick SteinhardtOct 14, 2024
  9. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 11, 2024
  10. karthik nayakOct 11, 2024
  11. Patrick SteinhardtOct 14, 2024
  12. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 11, 2024
  13. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 11, 2024
  14. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  15. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  16. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  17. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 11, 2024
  18. karthik nayakOct 11, 2024
  19. Patrick SteinhardtOct 14, 2024
  20. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 14, 2024
  21. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 14, 2024
  22. Taylor BlauOct 14, 2024
  23. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 14, 2024
  24. Taylor BlauOct 14, 2024
  25. Patrick SteinhardtOct 15, 2024
  26. Taylor BlauOct 15, 2024
  27. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 14, 2024
  28. Taylor BlauOct 14, 2024
  29. Patrick SteinhardtOct 15, 2024
  30. Eric SunshineOct 15, 2024
  31. Taylor BlauOct 15, 2024
  32. Patrick SteinhardtOct 16, 2024
  33. Taylor BlauOct 16, 2024
  34. Patrick SteinhardtOct 17, 2024
  35. Taylor BlauOct 17, 2024
  36. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 14, 2024
  37. Taylor BlauOct 14, 2024
  38. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 14, 2024
  39. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 14, 2024
  40. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  41. Taylor BlauOct 14, 2024
  42. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  43. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  44. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 14, 2024
  45. Taylor BlauOct 14, 2024
  46. Patrick SteinhardtOct 15, 2024
  47. shejialuoOct 15, 2024
  48. Patrick SteinhardtOct 15, 2024
  49. shejialuoOct 15, 2024
  50. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 17, 2024
  51. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 17, 2024
  52. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 17, 2024
  53. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 17, 2024
  54. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 17, 2024
  55. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 17, 2024
  56. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 17, 2024
  57. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  58. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  59. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  60. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 17, 2024
  61. Taylor BlauOct 17, 2024
  62. karthik nayakOct 18, 2024
  63. Taylor BlauOct 18, 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.