Re: [PATCH V2 3/3] strbuf: allow to use preallocated memory

[Michael Haggerty <mhagger@alum.mit.edu>]

On 06/07/2016 11:06 AM, William Duclot wrote:
> [...]
> The "fixed" feature was aimed to allow the users to use strbuf with
> strings that they doesn't own themselves (a function parameter for
> example). From Michael example in the original mail:
> 
> void f(char *path_buf, size_t path_buf_len)
> {
>     struct strbuf path;
>     strbuf_wrap_fixed(&path, path_buf,
>     strlen(path_buf),
>     path_buf_len);
>     ...

I also thought that "fixed" strbufs would be useful in cases where you
*know* what size string you need and only want a strbuf wrapper because
it offers a lot of convenience functions. Nowadays we would do that
using something like

> static int feed_object(const unsigned char *sha1, int fd, int negative)
> {
> 	char buf[42];
> 
> 	if (negative && !has_sha1_file(sha1))
> 		return 1;
> 
> 	memcpy(buf + negative, sha1_to_hex(sha1), 40);
> 	if (negative)
> 		buf[0] = '^';
> 	buf[40 + negative] = '\n';
> 	return write_or_whine(fd, buf, 41 + negative, "send-pack: send refs");
> }

Instead, one could write

> static int feed_object(const unsigned char *sha1, int fd, int negative)
> {
> 	char buf[GIT_SHA1_HEXSZ + 2];
> 	struct strbuf line = WRAPPED_FIXED_STRBUF(buf);
> 
> 	if (negative && !has_sha1_file(sha1))
> 		return 1;
> 
> 	if (negative)
> 		strbuf_addch(&line, '^');
> 	strbuf_add(&line, sha1_to_hex(sha1), GIT_SHA1_HEXSZ);
> 	strbuf_addch(&line, '\n');
> 	return write_or_whine(fd, line.buf, line.len, "send-pack: send refs");
> }

* It's a little less manual bookkeeping, and thus less error-prone,
  than the current code.

* If somebody decides to add another character to the line but
  forgets to increase the allocation size, the code dies in testing
  rather than (a) overflowing the buffer, like the current
  code, or (b) silently becoming less performant, as if it used a
  preallocated but non-fixed strbuf.

* There's no need to strbuf_release() (which can be convenient in
  a function with multiple exit paths).

I don't know whether this particular function should be rewritten; I'm
just giving an example of the type of scenario where I think it could be
useful.

In a world without fixed strbufs, what would one use in this situation?

Michael