Merge branch 'ps/reftable-strbuf' into ps/reftable-detach

* ps/reftable-strbuf:
  reftable: handle trivial `reftable_buf` errors
  reftable/stack: adapt `stack_filename()` to handle allocation failures
  reftable/record: adapt `reftable_record_key()` to handle allocation failures
  reftable/stack: adapt `format_name()` to handle allocation failures
  t/unit-tests: check for `reftable_buf` allocation errors
  reftable/blocksource: adapt interface name
  reftable: convert from `strbuf` to `reftable_buf`
  reftable/basics: provide new `reftable_buf` interface
  reftable: stop using `strbuf_addf()`
  reftable: stop using `strbuf_addbuf()`

Author: ttaylorr

reftable/basics.c

@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at
 #define REFTABLE_ALLOW_BANNED_ALLOCATORS
 #include "basics.h"
 #include "reftable-basics.h"
+#include "reftable-error.h"
 
 static void *(*reftable_malloc_ptr)(size_t sz);
 static void *(*reftable_realloc_ptr)(void *, size_t);
@@ -69,6 +70,79 @@ void reftable_set_alloc(void *(*malloc)(size_t),
 	reftable_free_ptr = free;
 }
 
+void reftable_buf_init(struct reftable_buf *buf)
+{
+	struct reftable_buf empty = REFTABLE_BUF_INIT;
+	*buf = empty;
+}
+
+void reftable_buf_release(struct reftable_buf *buf)
+{
+	reftable_free(buf->buf);
+	reftable_buf_init(buf);
+}
+
+void reftable_buf_reset(struct reftable_buf *buf)
+{
+	if (buf->alloc) {
+		buf->len = 0;
+		buf->buf[0] = '\0';
+	}
+}
+
+int reftable_buf_setlen(struct reftable_buf *buf, size_t len)
+{
+	if (len > buf->len)
+		return -1;
+	if (len == buf->len)
+		return 0;
+	buf->buf[len] = '\0';
+	buf->len = len;
+	return 0;
+}
+
+int reftable_buf_cmp(const struct reftable_buf *a, const struct reftable_buf *b)
+{
+	size_t len = a->len < b->len ? a->len : b->len;
+	if (len) {
+		int cmp = memcmp(a->buf, b->buf, len);
+		if (cmp)
+			return cmp;
+	}
+	return a->len < b->len ? -1 : a->len != b->len;
+}
+
+int reftable_buf_add(struct reftable_buf *buf, const void *data, size_t len)
+{
+	size_t newlen = buf->len + len;
+
+	if (newlen + 1 > buf->alloc) {
+		char *reallocated = buf->buf;
+		REFTABLE_ALLOC_GROW(reallocated, newlen + 1, buf->alloc);
+		if (!reallocated)
+			return REFTABLE_OUT_OF_MEMORY_ERROR;
+		buf->buf = reallocated;
+	}
+
+	memcpy(buf->buf + buf->len, data, len);
+	buf->buf[newlen] = '\0';
+	buf->len = newlen;
+
+	return 0;
+}
+
+int reftable_buf_addstr(struct reftable_buf *buf, const char *s)
+{
+	return reftable_buf_add(buf, s, strlen(s));
+}
+
+char *reftable_buf_detach(struct reftable_buf *buf)
+{
+	char *result = buf->buf;
+	reftable_buf_init(buf);
+	return result;
+}
+
 void put_be24(uint8_t *out, uint32_t i)
 {
 	out[0] = (uint8_t)((i >> 16) & 0xff);
@@ -186,7 +260,7 @@ int names_equal(const char **a, const char **b)
 	return a[i] == b[i];
 }
 
-int common_prefix_size(struct strbuf *a, struct strbuf *b)
+int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b)
 {
 	int p = 0;
 	for (; p < a->len && p < b->len; p++) {

reftable/basics.h

@@ -16,6 +16,64 @@ license that can be found in the LICENSE file or at
 #include "system.h"
 #include "reftable-basics.h"
 
+struct reftable_buf {
+	size_t alloc;
+	size_t len;
+	char *buf;
+};
+#define REFTABLE_BUF_INIT { 0 }
+
+/*
+ * Initialize the buffer such that it is ready for use. This is equivalent to
+ * using REFTABLE_BUF_INIT for stack-allocated variables.
+ */
+void reftable_buf_init(struct reftable_buf *buf);
+
+/*
+ * Release memory associated with the buffer. The buffer is reinitialized such
+ * that it can be reused for subsequent operations.
+ */
+void reftable_buf_release(struct reftable_buf *buf);
+
+/*
+ * Reset the buffer such that it is effectively empty, without releasing the
+ * memory that this structure holds on to. This is equivalent to calling
+ * `reftable_buf_setlen(buf, 0)`.
+ */
+void reftable_buf_reset(struct reftable_buf *buf);
+
+/*
+ * Trim the buffer to a shorter length by updating the `len` member and writing
+ * a NUL byte to `buf[len]`. Returns 0 on success, -1 when `len` points outside
+ * of the array.
+ */
+int reftable_buf_setlen(struct reftable_buf *buf, size_t len);
+
+/*
+ * Lexicographically compare the two buffers. Returns 0 when both buffers have
+ * the same contents, -1 when `a` is lexicographically smaller than `b`, and 1
+ * otherwise.
+ */
+int reftable_buf_cmp(const struct reftable_buf *a, const struct reftable_buf *b);
+
+/*
+ * Append `len` bytes from `data` to the buffer. This function works with
+ * arbitrary byte sequences, including ones that contain embedded NUL
+ * characters. As such, we use `void *` as input type. Returns 0 on success,
+ * REFTABLE_OUT_OF_MEMORY_ERROR on allocation failure.
+ */
+int reftable_buf_add(struct reftable_buf *buf, const void *data, size_t len);
+
+/* Equivalent to `reftable_buf_add(buf, s, strlen(s))`. */
+int reftable_buf_addstr(struct reftable_buf *buf, const char *s);
+
+/*
+ * Detach the buffer from the structure such that the underlying memory is now
+ * owned by the caller. The buffer is reinitialized such that it can be reused
+ * for subsequent operations.
+ */
+char *reftable_buf_detach(struct reftable_buf *buf);
+
 /* Bigendian en/decoding of integers */
 
 void put_be24(uint8_t *out, uint32_t i);
@@ -88,8 +146,7 @@ char *reftable_strdup(const char *str);
 #endif
 
 /* Find the longest shared prefix size of `a` and `b` */
-struct strbuf;
-int common_prefix_size(struct strbuf *a, struct strbuf *b);
+int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);
 
 int hash_size(uint32_t id);
 

reftable/block.c

@@ -38,9 +38,11 @@ int footer_size(int version)
 }
 
 static int block_writer_register_restart(struct block_writer *w, int n,
-					 int is_restart, struct strbuf *key)
+					 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;
 	}
@@ -59,8 +61,11 @@ static int block_writer_register_restart(struct block_writer *w, int n,
 
 	w->next += n;
 
-	strbuf_reset(&w->last_key);
-	strbuf_addbuf(&w->last_key, key);
+	reftable_buf_reset(&w->last_key);
+	err = reftable_buf_add(&w->last_key, key->buf, key->len);
+	if (err < 0)
+		return err;
+
 	w->entries++;
 	return 0;
 }
@@ -98,8 +103,8 @@ uint8_t block_writer_type(struct block_writer *bw)
    empty key. */
 int block_writer_add(struct block_writer *w, struct reftable_record *rec)
 {
-	struct strbuf empty = STRBUF_INIT;
-	struct strbuf last =
+	struct reftable_buf empty = REFTABLE_BUF_INIT;
+	struct reftable_buf last =
 		w->entries % w->restart_interval == 0 ? empty : w->last_key;
 	struct string_view out = {
 		.buf = w->buf + w->next,
@@ -109,31 +114,38 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)
 	struct string_view start = out;
 
 	int is_restart = 0;
-	struct strbuf key = STRBUF_INIT;
+	struct reftable_buf key = REFTABLE_BUF_INIT;
 	int n = 0;
-	int err = -1;
+	int err;
+
+	err = reftable_record_key(rec, &key);
+	if (err < 0)
+		goto done;
 
-	reftable_record_key(rec, &key);
 	if (!key.len) {
 		err = REFTABLE_API_ERROR;
 		goto done;
 	}
 
 	n = reftable_encode_key(&is_restart, out, last, key,
 				reftable_record_val_type(rec));
-	if (n < 0)
+	if (n < 0) {
+		err = -1;
 		goto done;
+	}
 	string_view_consume(&out, n);
 
 	n = reftable_record_encode(rec, out, w->hash_size);
-	if (n < 0)
+	if (n < 0) {
+		err = -1;
 		goto done;
+	}
 	string_view_consume(&out, n);
 
 	err = block_writer_register_restart(w, start.len - out.len, is_restart,
 					    &key);
 done:
-	strbuf_release(&key);
+	reftable_buf_release(&key);
 	return err;
 }
 
@@ -325,7 +337,7 @@ uint8_t block_reader_type(const struct block_reader *r)
 	return r->block.data[r->header_off];
 }
 
-int block_reader_first_key(const struct block_reader *br, struct strbuf *key)
+int block_reader_first_key(const struct block_reader *br, struct reftable_buf *key)
 {
 	int off = br->header_off + 4, n;
 	struct string_view in = {
@@ -334,7 +346,7 @@ int block_reader_first_key(const struct block_reader *br, struct strbuf *key)
 	};
 	uint8_t extra = 0;
 
-	strbuf_reset(key);
+	reftable_buf_reset(key);
 
 	n = reftable_decode_key(key, &extra, in);
 	if (n < 0)
@@ -355,13 +367,13 @@ void block_iter_seek_start(struct block_iter *it, const struct block_reader *br)
 	it->block = br->block.data;
 	it->block_len = br->block_len;
 	it->hash_size = br->hash_size;
-	strbuf_reset(&it->last_key);
+	reftable_buf_reset(&it->last_key);
 	it->next_off = br->header_off + 4;
 }
 
 struct restart_needle_less_args {
 	int error;
-	struct strbuf needle;
+	struct reftable_buf needle;
 	const struct block_reader *reader;
 };
 
@@ -433,7 +445,7 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)
 
 void block_iter_reset(struct block_iter *it)
 {
-	strbuf_reset(&it->last_key);
+	reftable_buf_reset(&it->last_key);
 	it->next_off = 0;
 	it->block = NULL;
 	it->block_len = 0;
@@ -442,12 +454,12 @@ void block_iter_reset(struct block_iter *it)
 
 void block_iter_close(struct block_iter *it)
 {
-	strbuf_release(&it->last_key);
-	strbuf_release(&it->scratch);
+	reftable_buf_release(&it->last_key);
+	reftable_buf_release(&it->scratch);
 }
 
 int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,
-			struct strbuf *want)
+			struct reftable_buf *want)
 {
 	struct restart_needle_less_args args = {
 		.needle = *want,
@@ -522,6 +534,10 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,
 			goto done;
 		}
 
+		err = reftable_record_key(&rec, &it->last_key);
+		if (err < 0)
+			goto done;
+
 		/*
 		 * Check whether the current key is greater or equal to the
 		 * sought-after key. In case it is greater we know that the
@@ -536,8 +552,7 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,
 		 * to `last_key` now, and naturally all keys share a prefix
 		 * with themselves.
 		 */
-		reftable_record_key(&rec, &it->last_key);
-		if (strbuf_cmp(&it->last_key, want) >= 0) {
+		if (reftable_buf_cmp(&it->last_key, want) >= 0) {
 			it->next_off = prev_off;
 			goto done;
 		}
@@ -554,7 +569,7 @@ void block_writer_release(struct block_writer *bw)
 	REFTABLE_FREE_AND_NULL(bw->zstream);
 	REFTABLE_FREE_AND_NULL(bw->restarts);
 	REFTABLE_FREE_AND_NULL(bw->compressed);
-	strbuf_release(&bw->last_key);
+	reftable_buf_release(&bw->last_key);
 	/* the block is not owned. */
 }
 

reftable/block.h

@@ -38,7 +38,7 @@ struct block_writer {
 	uint32_t restart_len;
 	uint32_t restart_cap;
 
-	struct strbuf last_key;
+	struct reftable_buf last_key;
 	int entries;
 };
 
@@ -98,7 +98,7 @@ void block_reader_release(struct block_reader *br);
 uint8_t block_reader_type(const struct block_reader *r);
 
 /* Decodes the first key in the block */
-int block_reader_first_key(const struct block_reader *br, struct strbuf *key);
+int block_reader_first_key(const struct block_reader *br, struct reftable_buf *key);
 
 /* Iterate over entries in a block */
 struct block_iter {
@@ -109,21 +109,21 @@ struct block_iter {
 	int hash_size;
 
 	/* key for last entry we read. */
-	struct strbuf last_key;
-	struct strbuf scratch;
+	struct reftable_buf last_key;
+	struct reftable_buf scratch;
 };
 
 #define BLOCK_ITER_INIT { \
-	.last_key = STRBUF_INIT, \
-	.scratch = STRBUF_INIT, \
+	.last_key = REFTABLE_BUF_INIT, \
+	.scratch = REFTABLE_BUF_INIT, \
 }
 
 /* Position `it` at start of the block */
 void block_iter_seek_start(struct block_iter *it, const struct block_reader *br);
 
 /* Position `it` to the `want` key in the block */
 int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,
-			struct strbuf *want);
+			struct reftable_buf *want);
 
 /* return < 0 for error, 0 for OK, > 0 for EOF. */
 int block_iter_next(struct block_iter *it, struct reftable_record *rec);