Re: [regression] Re: [PATCHv2 10/15] drop length limitations on gecos-derived names and emails

Jeff King [off-list ref] writes:

(with a proper commit message, of course).

Will queue this one, to be merged to 'maint' and 'master'.

-- >8 --
From: Jonathan Nieder <redacted>
Date: Thu, 24 Jan 2013 15:21:46 -0800
Subject: [PATCH] ident: do not drop username when reading from /etc/mailname

An earlier conversion from fgets() to strbuf_getline() in the
codepath to read from /etc/mailname to learn the default host-part
of the ident e-mail address forgot that strbuf_getline() stores the
line at the beginning of the buffer just like fgets().

The "username@" the caller has prepared in the strbuf, expecting the
function to append the host-part to it, was lost because of this.

Reported-by: Mihai Rusu <redacted>
Signed-off-by: Jonathan Nieder <redacted>
Acked-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 ident.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ident.c b/ident.c
index 484e0a9..fb4cc72 100644
--- a/ident.c
+++ b/ident.c

@@ -41,6 +41,7 @@ static void copy_gecos(const struct passwd *w, struct strbuf *name)
 static int add_mailname_host(struct strbuf *buf)
 {
 	FILE *mailname;
+	struct strbuf mailnamebuf = STRBUF_INIT;
 
 	mailname = fopen("/etc/mailname", "r");
 	if (!mailname) {

@@ -49,14 +50,17 @@ static int add_mailname_host(struct strbuf *buf)
 				strerror(errno));
 		return -1;
 	}
-	if (strbuf_getline(buf, mailname, '\n') == EOF) {
+	if (strbuf_getline(&mailnamebuf, mailname, '\n') == EOF) {
 		if (ferror(mailname))
 			warning("cannot read /etc/mailname: %s",
 				strerror(errno));
+		strbuf_release(&mailnamebuf);
 		fclose(mailname);
 		return -1;
 	}
 	/* success! */
+	strbuf_addbuf(buf, &mailnamebuf);
+	strbuf_release(&mailnamebuf);
 	fclose(mailname);
 	return 0;
 }

-- 
1.8.1.1.525.gdace530

On Fri, Jan 25, 2013 at 10:46:48AM -0800, Junio C Hamano wrote:

Will queue this one, to be merged to 'maint' and 'master'.

-- >8 --
From: Jonathan Nieder <redacted>
Date: Thu, 24 Jan 2013 15:21:46 -0800
Subject: [PATCH] ident: do not drop username when reading from /etc/mailname

Thanks, looks fine to me (and thanks to Jonathan). One nit:

-	if (strbuf_getline(buf, mailname, '\n') == EOF) {
+	if (strbuf_getline(&mailnamebuf, mailname, '\n') == EOF) {
 		if (ferror(mailname))
 			warning("cannot read /etc/mailname: %s",
 				strerror(errno));
+		strbuf_release(&mailnamebuf);
 		fclose(mailname);
 		return -1;
 	}

This strbuf_release is unnecessary, as an EOF return by definition means
we did not read anything. I don't mind it as a defensive measure,
though, in case the strbuf implementation changes to pre-allocate.

-Peff