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

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Jun 8, 2016, 16:20 UTC
Message-ID: <575845D9.2010604@alum.mit.edu>
In-Reply-To: <20160607090653.GA4665@Messiaen>

On 06/07/2016 11:06 AM, William Duclot wrote:

Show 12 quoted lines

> [...]
> 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

Show 13 quoted lines

> 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

Show 14 quoted lines

> 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