On Wed, Oct 16, 2024 at 10:42:44AM +0200, Patrick Steinhardt wrote:

On Tue, Oct 15, 2024 at 03:27:29PM -0400, Taylor Blau wrote:

quoted

On Tue, Oct 15, 2024 at 01:10:59AM -0400, Eric Sunshine wrote:

quoted

On Tue, Oct 15, 2024 at 12:38 AM Patrick Steinhardt [off-list ref] wrote:

quoted

On Mon, Oct 14, 2024 at 06:34:55PM -0400, Taylor Blau wrote:

quoted

On Mon, Oct 14, 2024 at 03:02:24PM +0200, Patrick Steinhardt wrote:

quoted

+/*
+ * Add the given bytes to the buffer. 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);

Is there a reason that data is a void-pointer here and not a const char
*?

Only that it emulates `strbuf_add()`, which also uses a void pointer.

The reason for that is because strbuf is a generic byte-array which
may contain embedded NULs, and the `const void *` plus `len`
emphasizes this property, whereas `const char *` would imply a
C-string with no embedded NULs.

Thanks, that was the explanation I was missing. Perhaps it is worth
re-stating in the commit message here to avoid confusing readers like I
was when I first read Patrick's patch ;-).

Does it make sense to explicitly state how the interfaces look like
though? I don't do that for the other functions either, and for most of
the part I just reuse the exact same function arguments as we had with
the strbuf interface.

I don't feel very strongly about it, but I had suggested it because my
initial read of this patch confused me, and I had wondered if others may
be similarly confused.

For what it's worth, I was thinking something on the order of the
following added to the patch message:

    Note that the reftable_buf_add() function intentionally takes a "const
    void *" instead of a "const char *" (as does its strbuf counterpart,
    strbuf_add()) to emphasize that the buffer may contain NUL characters.

But, as I said, I don't feel very strongly about it.

Thanks,
Taylor