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

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

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

> 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