Re: [PATCH] strbuf: allocate enough space when strbuf_setlen() is called first time

[Junio C Hamano <gitster@pobox.com>]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Subject: strbuf: clarify assertion in strbuf_setlen()
>
> Commit a8f3e2219 introduced the strbuf_grow() call to strbuf_setlen() to
> make ensure that there was at least one byte available to write the
> mandatory trailing NUL, even for previously unallocated strbufs.
>
> Then b315c5c0 added strbuf_slopbuf for the same reason, only globally for
> all uses of strbufs.
>
> Thus the strbuf_grow() call can be removed now.  This avoids readers of
> strbuf.h from mistakenly thinking that strbuf_setlen() can be used to
> extend a strbuf.
>
> The following assert() needs to be changed to cope with the fact that
> sb->alloc can now be zero, which is OK as long as len is also zero.  As
> suggested by Junio, use the chance to convert it to a die() with a short
> explanatory message.  The pattern of 'die("BUG: ...")' is already used in
> strbuf.c.
>
> This was the only assert() in strbuf.[ch], so assert.h doesn't have to be
> included anymore either.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>

Very nicely explained.  I wish everybody wrote his log message as clearly
as this.

Thanks, will queue.

> ---
>  strbuf.h |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/strbuf.h b/strbuf.h
> index 07060ce..9e6d9fa 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -3,8 +3,6 @@
>  
>  /* See Documentation/technical/api-strbuf.txt */
>  
> -#include <assert.h>
> -
>  extern char strbuf_slopbuf[];
>  struct strbuf {
>  	size_t alloc;
> @@ -33,9 +31,8 @@ static inline size_t strbuf_avail(const struct strbuf *sb) {
>  extern void strbuf_grow(struct strbuf *, size_t);
>  
>  static inline void strbuf_setlen(struct strbuf *sb, size_t len) {
> -	if (!sb->alloc)
> -		strbuf_grow(sb, 0);
> -	assert(len < sb->alloc);
> +	if (len > (sb->alloc ? sb->alloc - 1 : 0))
> +		die("BUG: strbuf_setlen() beyond buffer");
>  	sb->len = len;
>  	sb->buf[len] = '\0';
>  }