diff mbox series

[03/10] reftable/basics: provide new `reftable_buf` interface

Message ID 24e31619b936166404b801dde0e2bca478328386.1728629612.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series reftable: stop using `struct strbuf` | expand

Commit Message

Patrick Steinhardt Oct. 11, 2024, 6:54 a.m. UTC
Implement a new `reftable_buf` interface that will replace Git's own
`strbuf` interface. This is done due to three reasons:

  - The `strbuf` interfaces do not handle memory allocation failures and
    instead causes us to die. This is okay in the context of Git, but is
    not context of the reftable library, which is supposed to be usable
    by third-party applications.

  - The `strbuf` interface is quite deeply tied into Git, which makes it
    hard to use the reftable library as a standalone library. Any
    dependent would have to carefully extract the relevant parts of it
    to make things work, which is not all that sensible.

  - The `strbuf` interface does not use the pluggable allocators that
    can be set up via `refatble_set_alloc()`.

So we have good reasons to use our own type, and the implementation is
rather trivial. Implement our own type. Conversion of the reftable
library will be handled in subsequent commits.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/basics.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
 reftable/basics.h | 16 +++++++++++
 2 files changed, 89 insertions(+)

Comments

karthik nayak Oct. 11, 2024, 10:03 a.m. UTC | #1
Patrick Steinhardt <ps@pks.im> writes:

> Implement a new `reftable_buf` interface that will replace Git's own
> `strbuf` interface. This is done due to three reasons:
>
>   - The `strbuf` interfaces do not handle memory allocation failures and
>     instead causes us to die. This is okay in the context of Git, but is
>     not context of the reftable library, which is supposed to be usable

s/not/not in the/

>     by third-party applications.
>
>   - The `strbuf` interface is quite deeply tied into Git, which makes it
>     hard to use the reftable library as a standalone library. Any
>     dependent would have to carefully extract the relevant parts of it
>     to make things work, which is not all that sensible.
>
>   - The `strbuf` interface does not use the pluggable allocators that
>     can be set up via `refatble_set_alloc()`.
>
> So we have good reasons to use our own type, and the implementation is
> rather trivial. Implement our own type. Conversion of the reftable
> library will be handled in subsequent commits.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>

[snip]

> diff --git a/reftable/basics.h b/reftable/basics.h
> index 4c9ef0fe6c5..4cf3f0e7593 100644
> --- a/reftable/basics.h
> +++ b/reftable/basics.h
> @@ -16,6 +16,22 @@ 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 }
> +
> +void reftable_buf_init(struct reftable_buf *buf);
> +void reftable_buf_release(struct reftable_buf *buf);
> +void reftable_buf_reset(struct reftable_buf *buf);
> +int reftable_buf_setlen(struct reftable_buf *buf, size_t len);
> +int reftable_buf_cmp(const struct reftable_buf *a, const struct reftable_buf *b);
> +int reftable_buf_add(struct reftable_buf *buf, const void *data, size_t len);
> +int reftable_buf_addstr(struct reftable_buf *buf, const char *s);
> +char *reftable_buf_detach(struct reftable_buf *buf);
> +

Nit: would be nice to have some comments explaining the functions here.
I know most of them are self-explanatory and similar to strbuf, but
since this is supposed to be isolated, it would be nice.

[snip]
Patrick Steinhardt Oct. 14, 2024, 1:09 p.m. UTC | #2
On Fri, Oct 11, 2024 at 05:03:39AM -0500, karthik nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/reftable/basics.h b/reftable/basics.h
> > index 4c9ef0fe6c5..4cf3f0e7593 100644
> > --- a/reftable/basics.h
> > +++ b/reftable/basics.h
> > @@ -16,6 +16,22 @@ 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 }
> > +
> > +void reftable_buf_init(struct reftable_buf *buf);
> > +void reftable_buf_release(struct reftable_buf *buf);
> > +void reftable_buf_reset(struct reftable_buf *buf);
> > +int reftable_buf_setlen(struct reftable_buf *buf, size_t len);
> > +int reftable_buf_cmp(const struct reftable_buf *a, const struct reftable_buf *b);
> > +int reftable_buf_add(struct reftable_buf *buf, const void *data, size_t len);
> > +int reftable_buf_addstr(struct reftable_buf *buf, const char *s);
> > +char *reftable_buf_detach(struct reftable_buf *buf);
> > +
> 
> Nit: would be nice to have some comments explaining the functions here.
> I know most of them are self-explanatory and similar to strbuf, but
> since this is supposed to be isolated, it would be nice.

Fair enough. I was being lazy here because the code is internal, only.
But that's not really a good excuse, I guess.

Patrick
diff mbox series

Patch

diff --git a/reftable/basics.c b/reftable/basics.c
index 9a949e5cf80..a6abefe7f5d 100644
--- a/reftable/basics.c
+++ b/reftable/basics.c
@@ -69,6 +69,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 -1;
+		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);
diff --git a/reftable/basics.h b/reftable/basics.h
index 4c9ef0fe6c5..4cf3f0e7593 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -16,6 +16,22 @@  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 }
+
+void reftable_buf_init(struct reftable_buf *buf);
+void reftable_buf_release(struct reftable_buf *buf);
+void reftable_buf_reset(struct reftable_buf *buf);
+int reftable_buf_setlen(struct reftable_buf *buf, size_t len);
+int reftable_buf_cmp(const struct reftable_buf *a, const struct reftable_buf *b);
+int reftable_buf_add(struct reftable_buf *buf, const void *data, size_t len);
+int reftable_buf_addstr(struct reftable_buf *buf, const char *s);
+char *reftable_buf_detach(struct reftable_buf *buf);
+
 /* Bigendian en/decoding of integers */
 
 void put_be24(uint8_t *out, uint32_t i);