Re: [RFC] cocci: .buf in a strbuf object can never be NULL

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 21, 2026, 16:24 UTC
Message-ID: <xmqqqzpdb172.fsf@gitster.g>
In-Reply-To: <xmqqcy0zii0s.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 13 quoted lines

> diff --git c/contrib/coccinelle/strbuf.cocci w/contrib/coccinelle/strbuf.cocci
> index 5f06105df6..3dc5cd02a3 100644
> --- c/contrib/coccinelle/strbuf.cocci
> +++ w/contrib/coccinelle/strbuf.cocci
> ...
> +@@
> +identifier funcname != { strbuf_getwholeline, parse_list_objects_filter };
> +struct strbuf SB;
> +@@
> +  funcname(...) {<...
> +- !SB.buf
> ++ 0
> +  ...>}

Here is my second try. strbuf_getwholeline() does not have to break strbuf invariants even tentatively. We just grab the guts of sb, let getdelim() possibly reallocate, and then return it in the normal case.

In the EOF code path, the only special thing we need is when we started with slopbuf[] and getdelim() allocated some bytes yet returned EOF. We are expected to free it before returning.

By the way, the big comment about xrealloc() in the middle, most of which is outside the post-context of the first hunk, should be updated, as our xrealloc() do not aggressively try to recover these days, if I understand correctly. I left it outside the scope of this patch, whose sole focus is to reduce the number of places in the codebase that check if sb->buf is NULL.

 strbuf.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git c/strbuf.c w/strbuf.c
index 3939863cf3..89933c3814 100644
--- c/strbuf.c
+++ w/strbuf.c
@@ -632,24 +632,26 @@ int strbuf_getcwd(struct strbuf *sb)
 int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
 {
 	ssize_t r;
+	char *buf = sb->buf;
+	size_t alloc = sb->alloc;
 
 	if (feof(fp))
 		return EOF;
 
-	strbuf_reset(sb);
-
 	/* Translate slopbuf to NULL, as we cannot call realloc on it */
-	if (!sb->alloc)
-		sb->buf = NULL;
+	if (!alloc)
+		buf = NULL;
 	errno = 0;
-	r = getdelim(&sb->buf, &sb->alloc, term, fp);
+	r = getdelim(&buf, &alloc, term, fp);
 
 	if (r > 0) {
+		sb->buf = buf;
+		sb->alloc = alloc;
 		sb->len = r;
 		return 0;
 	}
-	assert(r == -1);
 
+	assert(r == -1);
 	/*
 	 * Normally we would have called xrealloc, which will try to free
 	 * memory and recover. But we have no way to tell getdelim() to do so.
@@ -664,15 +666,16 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
 	if (errno == ENOMEM)
 		die("Out of memory, getdelim failed");
 
-	/*
-	 * Restore strbuf invariants; if getdelim left us with a NULL pointer,
-	 * we can just re-init, but otherwise we should make sure that our
-	 * length is empty, and that the result is NUL-terminated.
+	/* 
+	 * If getdelim() allocated when we had no allocation, free it.
+	 */
+	if (!alloc)
+		free(buf);
+
+	/* 
+	 * We haven't touched sb at all; as with the initial "were we
+	 * already at EOF?" case, return EOF without touching sb.
 	 */
-	if (!sb->buf)
-		strbuf_init(sb, 0);
-	else
-		strbuf_reset(sb);
 	return EOF;
 }
 #else