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

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

Jeff King <peff@peff.net> writes:

> I'm not quite sure what you mean. The function right now looks like:
> ...
> But it's not quite enough to just do:
>
>   free(buf);
>
> Because "buf" is a copy of sb->buf,...

I may have phrased the idea very poorly.  In short, the core of the
idea is that we do not have to use the original content of the
strbuf at all.  I.e., "buf" does not have to be anything related to
sb->buf.

In the following patch, I am _removing_ strbuf_reset() near the
beginning, but replacing it with strbuf_release() may illustrate the
idea more clearly.  I didn't do so primarily because the first thing
strbuf_attach() does is to call strbuf_release(), so the call would
be redundant in the normal code flow.

When getdelim() did not return anything positive, after asserting
the return value is -1 (i.e., EOF), we only need to return EOF while
emptying the caller-supplied strbuf.  As the "char *buf" we have and
passed to getdelim() never had anything to do with caller-supplied
strbuf *sb, there is no need to worry about slopbuf or anything
associated with it.

 strbuf.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git c/strbuf.c w/strbuf.c
index 3939863cf3..ef61bd5b14 100644
--- c/strbuf.c
+++ w/strbuf.c
@@ -631,21 +631,20 @@ int strbuf_getcwd(struct strbuf *sb)
 #ifdef HAVE_GETDELIM
 int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
 {
+	size_t alloc;
+	char *buf;
 	ssize_t r;
 
 	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;
+	buf = NULL;
+	alloc = 0;
 	errno = 0;
-	r = getdelim(&sb->buf, &sb->alloc, term, fp);
+	r = getdelim(&buf, &alloc, term, fp);
 
 	if (r > 0) {
-		sb->len = r;
+		strbuf_attach(sb, buf, (size_t) r, alloc);
 		return 0;
 	}
 	assert(r == -1);
@@ -664,15 +663,14 @@ 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.
+	/* 
+	 * We got an EOF.  If getdelim() allocated any memory, we
+	 * would return that to the system.
 	 */
-	if (!sb->buf)
-		strbuf_init(sb, 0);
-	else
-		strbuf_reset(sb);
+	free(buf);
+
+	/* And empty the strbuf */
+	strbuf_release(sb);
 	return EOF;
 }
 #else