All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Angus Hammond <angusgh@gmail.com>, git@vger.kernel.org
Subject: [PATCH 09/13] drop length limitations on gecos-derived names and emails
Date: Fri, 18 May 2012 19:20:39 -0400	[thread overview]
Message-ID: <20120518232039.GI30031@sigill.intra.peff.net> (raw)
In-Reply-To: <20120518230528.GA30510@sigill.intra.peff.net>

When we pull the user's name from the GECOS field of the
passwd file (or generate an email address based on their
username and hostname), we put the result into a
static buffer. While it's extremely unlikely that anybody
ever hit these limits (after all, in such a case their
parents must have hated them), we still had to deal with the
error cases in our code.

Converting these static buffers to strbufs lets us simplify
the code and drop some error messages from the documentation
that have confused some users.

The conversion is mostly mechanical: replace string copies
with strbuf equivalents, and access the strbuf.buf directly.
There are a few exceptions:

  - copy_gecos and copy_email are the big winners in code
    reduction (since they no longer have to manage the
    string length manually)

  - git_ident_config wants to replace old versions of
    the default name (e.g., if we read the config multiple
    times), so it must reset+add to the strbuf instead of
    just adding

Note that there is still one length limitation: the
gethostname interface requires us to provide a static
buffer, so we arbitrarily choose 1024 bytes for the
hostname.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-commit-tree.txt |   4 --
 Documentation/git-var.txt         |   4 --
 ident.c                           | 100 +++++++++++++++-----------------------
 3 files changed, 39 insertions(+), 69 deletions(-)

diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
index cfb9906..eb12b2d 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt
@@ -92,10 +92,6 @@ Diagnostics
 -----------
 You don't exist. Go away!::
     The passwd(5) gecos field couldn't be read
-Your parents must have hated you!::
-    The passwd(5) gecos field is longer than a giant static buffer.
-Your sysadmin must hate you!::
-    The passwd(5) name field is longer than a giant static buffer.
 
 Discussion
 ----------
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 988a323..3f703e3 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -63,10 +63,6 @@ Diagnostics
 -----------
 You don't exist. Go away!::
     The passwd(5) gecos field couldn't be read
-Your parents must have hated you!::
-    The passwd(5) gecos field is longer than a giant static buffer.
-Your sysadmin must hate you!::
-    The passwd(5) name field is longer than a giant static buffer.
 
 SEE ALSO
 --------
diff --git a/ident.c b/ident.c
index f44bcb3..73a06a1 100644
--- a/ident.c
+++ b/ident.c
@@ -7,9 +7,8 @@
  */
 #include "cache.h"
 
-#define MAX_GITNAME (1000)
-static char git_default_name[MAX_GITNAME];
-static char git_default_email[MAX_GITNAME];
+static struct strbuf git_default_name = STRBUF_INIT;
+static struct strbuf git_default_email = STRBUF_INIT;
 static char git_default_date[50];
 int user_ident_explicitly_given;
 
@@ -19,42 +18,27 @@ int user_ident_explicitly_given;
 #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)
 #endif
 
-static void copy_gecos(const struct passwd *w, char *name, size_t sz)
+static void copy_gecos(const struct passwd *w, struct strbuf *name)
 {
-	char *src, *dst;
-	size_t len, nlen;
-
-	nlen = strlen(w->pw_name);
+	char *src;
 
 	/* Traditionally GECOS field had office phone numbers etc, separated
 	 * with commas.  Also & stands for capitalized form of the login name.
 	 */
 
-	for (len = 0, dst = name, src = get_gecos(w); len < sz; src++) {
+	for (src = get_gecos(w); *src && *src != ','; src++) {
 		int ch = *src;
-		if (ch != '&') {
-			*dst++ = ch;
-			if (ch == 0 || ch == ',')
-				break;
-			len++;
-			continue;
-		}
-		if (len + nlen < sz) {
+		if (ch != '&')
+			strbuf_addch(name, ch);
+		else {
 			/* Sorry, Mr. McDonald... */
-			*dst++ = toupper(*w->pw_name);
-			memcpy(dst, w->pw_name + 1, nlen - 1);
-			dst += nlen - 1;
-			len += nlen;
+			strbuf_addch(name, toupper(*w->pw_name));
+			strbuf_addstr(name, w->pw_name + 1);
 		}
 	}
-	if (len < sz)
-		name[len] = 0;
-	else
-		die("Your parents must have hated you!");
-
 }
 
-static int add_mailname_host(char *buf, size_t len)
+static int add_mailname_host(struct strbuf *buf)
 {
 	FILE *mailname;
 
@@ -65,7 +49,7 @@ static int add_mailname_host(char *buf, size_t len)
 				strerror(errno));
 		return -1;
 	}
-	if (!fgets(buf, len, mailname)) {
+	if (strbuf_getline(buf, mailname, '\n') == EOF) {
 		if (ferror(mailname))
 			warning("cannot read /etc/mailname: %s",
 				strerror(errno));
@@ -77,78 +61,70 @@ static int add_mailname_host(char *buf, size_t len)
 	return 0;
 }
 
-static void add_domainname(char *buf, size_t len)
+static void add_domainname(struct strbuf *out)
 {
+	char buf[1024];
 	struct hostent *he;
-	size_t namelen;
 	const char *domainname;
 
-	if (gethostname(buf, len)) {
+	if (gethostname(buf, sizeof(buf))) {
 		warning("cannot get host name: %s", strerror(errno));
-		strlcpy(buf, "(none)", len);
+		strbuf_addstr(out, "(none)");
 		return;
 	}
-	namelen = strlen(buf);
-	if (memchr(buf, '.', namelen))
+	strbuf_addstr(out, buf);
+	if (strchr(buf, '.'))
 		return;
 
 	he = gethostbyname(buf);
-	buf[namelen++] = '.';
-	buf += namelen;
-	len -= namelen;
+	strbuf_addch(out, '.');
 	if (he && (domainname = strchr(he->h_name, '.')))
-		strlcpy(buf, domainname + 1, len);
+		strbuf_addstr(out, domainname + 1);
 	else
-		strlcpy(buf, "(none)", len);
+		strbuf_addstr(out, "(none)");
 }
 
-static void copy_email(const struct passwd *pw)
+static void copy_email(const struct passwd *pw, struct strbuf *email)
 {
 	/*
 	 * Make up a fake email address
 	 * (name + '@' + hostname [+ '.' + domainname])
 	 */
-	size_t len = strlen(pw->pw_name);
-	if (len > sizeof(git_default_email)/2)
-		die("Your sysadmin must hate you!");
-	memcpy(git_default_email, pw->pw_name, len);
-	git_default_email[len++] = '@';
-
-	if (!add_mailname_host(git_default_email + len,
-				sizeof(git_default_email) - len))
+	strbuf_addstr(email, pw->pw_name);
+	strbuf_addch(email, '@');
+
+	if (!add_mailname_host(email))
 		return;	/* read from "/etc/mailname" (Debian) */
-	add_domainname(git_default_email + len,
-			sizeof(git_default_email) - len);
+	add_domainname(email);
 }
 
 const char *ident_default_name(void)
 {
-	if (!git_default_name[0]) {
+	if (!git_default_name.len) {
 		struct passwd *pw = getpwuid(getuid());
 		if (!pw)
 			die("You don't exist. Go away!");
-		copy_gecos(pw, git_default_name, sizeof(git_default_name));
+		copy_gecos(pw, &git_default_name);
 	}
-	return git_default_name;
+	return git_default_name.buf;
 }
 
 const char *ident_default_email(void)
 {
-	if (!git_default_email[0]) {
+	if (!git_default_email.len) {
 		const char *email = getenv("EMAIL");
 
 		if (email && email[0]) {
-			strlcpy(git_default_email, email,
-				sizeof(git_default_email));
+			strbuf_addstr(&git_default_email, email);
 			user_ident_explicitly_given |= IDENT_MAIL_GIVEN;
 		} else {
 			struct passwd *pw = getpwuid(getuid());
 			if (!pw)
 				die("You don't exist. Go away!");
-			copy_email(pw);
+			copy_email(pw, &git_default_email);
 		}
 	}
-	return git_default_email;
+	return git_default_email.buf;
 }
 
 const char *ident_default_date(void)
@@ -323,7 +299,7 @@ const char *fmt_ident(const char *name, const char *email,
 		struct passwd *pw;
 
 		if (error_on_no_name) {
-			if (name == git_default_name)
+			if (name == git_default_name.buf)
 				fputs(env_hint, stderr);
 			die("empty ident %s <%s> not allowed", name, email);
 		}
@@ -393,7 +369,8 @@ int git_ident_config(const char *var, const char *value, void *data)
 	if (!strcmp(var, "user.name")) {
 		if (!value)
 			return config_error_nonbool(var);
-		strlcpy(git_default_name, value, sizeof(git_default_name));
+		strbuf_reset(&git_default_name);
+		strbuf_addstr(&git_default_name, value);
 		user_ident_explicitly_given |= IDENT_NAME_GIVEN;
 		return 0;
 	}
@@ -401,7 +378,8 @@ int git_ident_config(const char *var, const char *value, void *data)
 	if (!strcmp(var, "user.email")) {
 		if (!value)
 			return config_error_nonbool(var);
-		strlcpy(git_default_email, value, sizeof(git_default_email));
+		strbuf_reset(&git_default_email);
+		strbuf_addstr(&git_default_email, value);
 		user_ident_explicitly_given |= IDENT_MAIL_GIVEN;
 		return 0;
 	}
-- 
1.7.10.1.16.g53a707b

  parent reply	other threads:[~2012-05-18 23:20 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-10 19:06 [PATCH 1/2] Change error messages in ident.c Make error messages caused by failed reads of the /etc/passwd file easier to understand. Signed-off-by: Angus Hammond <angusgh@gmail.com> Angus Hammond
2012-05-10 19:06 ` [PATCH 2/2] Remove diagnostics section from commit-tree and var man pages New error messages shouldn't need explaining like the old ones did so just delete the diagnostics section of the man pages. " Angus Hammond
2012-05-10 19:21   ` Angus Hammond
2012-05-10 19:23 ` [PATCH 1/2] Change error messages in ident.c Jeff King
2012-05-10 19:56   ` Jeff King
2012-05-11 22:53     ` Junio C Hamano
2012-05-11 23:13       ` Jeff King
2012-05-14 16:28         ` [PATCH 1/2] drop length limitations on gecos-derived names and emails Jeff King
2012-05-14 17:05           ` Jeff King
2012-05-14 21:02           ` Jeff King
2012-05-14 21:13             ` Jeff King
2012-05-15  1:54               ` Jeff King
2012-05-15  2:32                 ` Jeff King
2012-05-15 15:03                 ` Junio C Hamano
2012-05-15 17:47                   ` Jeff King
2012-05-15 18:10                     ` Junio C Hamano
2012-05-18 23:05                       ` [PATCH 0/13] ident cleanups and bugfixes Jeff King
2012-05-18 23:07                         ` [PATCH 01/13] ident: split setup_ident into separate functions Jeff King
2012-05-18 23:09                         ` [PATCH 02/13] http-push: do not access git_default_email directly Jeff King
2012-05-18 23:10                         ` [PATCH 03/13] fmt-merge-msg: don't use static buffer in record_person Jeff King
2012-05-18 23:11                         ` [PATCH 04/13] move identity config parsing to ident.c Jeff King
2012-05-18 23:11                         ` [PATCH 05/13] move git_default_* variables " Jeff King
2012-05-21  4:07                           ` Junio C Hamano
2012-05-21  5:41                             ` Jeff King
2012-05-21  6:41                               ` Jeff King
2012-05-18 23:13                         ` [PATCH 06/13] format-patch: use default email for generating message ids Jeff King
2012-05-21  2:58                           ` Junio C Hamano
2012-05-21  6:36                             ` Jeff King
2012-05-18 23:14                         ` [PATCH 07/13] fmt_ident: drop IDENT_WARN_ON_NO_NAME code Jeff King
2012-05-18 23:19                         ` [PATCH 08/13] ident: don't write fallback username into git_default_name Jeff King
2012-05-21  2:54                           ` Junio C Hamano
2012-05-21  6:31                             ` Jeff King
2012-05-21  9:11                               ` Junio C Hamano
2012-05-21 23:09                                 ` [PATCHv2 0/15] ident cleanups git_default_name Jeff King
2012-05-21 23:09                                   ` [PATCHv2 01/15] ident: split setup_ident into separate functions Jeff King
2012-05-21 23:09                                   ` [PATCHv2 02/15] http-push: do not access git_default_email directly Jeff King
2012-05-21 23:09                                   ` [PATCHv2 03/15] fmt-merge-msg: don't use static buffer in record_person Jeff King
2012-05-21 23:09                                   ` [PATCHv2 04/15] move identity config parsing to ident.c Jeff King
2012-05-21 23:09                                   ` [PATCHv2 05/15] move git_default_* variables " Jeff King
2012-05-21 23:10                                   ` [PATCHv2 06/15] ident: trim trailing newline from /etc/mailname Jeff King
2012-05-21 23:10                                   ` [PATCHv2 07/15] format-patch: use default email for generating message ids Jeff King
2012-05-21 23:10                                   ` [PATCHv2 08/15] fmt_ident: drop IDENT_WARN_ON_NO_NAME code Jeff King
2012-05-21 23:10                                   ` [PATCHv2 09/15] ident: don't write fallback username into git_default_name Jeff King
2012-05-21 23:10                                   ` [PATCHv2 10/15] drop length limitations on gecos-derived names and emails Jeff King
2013-01-24 23:21                                     ` [regression] " Jonathan Nieder
2013-01-25  1:05                                       ` Jeff King
2013-01-25 18:46                                         ` Junio C Hamano
2013-01-25 22:10                                           ` Jeff King
2012-05-21 23:10                                   ` [PATCHv2 11/15] ident: report passwd errors with a more friendly message Jeff King
2012-05-21 23:10                                   ` [PATCHv2 12/15] ident: use full dns names to generate email addresses Jeff King
2012-05-21 23:10                                   ` [PATCHv2 13/15] ident: use a dynamic strbuf in fmt_ident Jeff King
2012-05-21 23:10                                   ` [PATCHv2 14/15] ident: trim whitespace from default name/email Jeff King
2012-05-22 16:55                                     ` Junio C Hamano
2012-05-22 17:12                                       ` Jeff King
2012-05-22 17:21                                         ` Junio C Hamano
2012-05-21 23:10                                   ` [PATCHv2 15/15] format-patch: refactor get_patch_filename Jeff King
2012-05-18 23:20                         ` Jeff King [this message]
2012-05-18 23:21                         ` [PATCH 10/13] ident: report passwd errors with a more friendly message Jeff King
2012-05-18 23:22                         ` [PATCH 11/13] ident: use full dns names to generate email addresses Jeff King
2012-05-18 23:23                         ` [PATCH 12/13] ident: use a dynamic strbuf in fmt_ident Jeff King
2012-05-18 23:24                         ` [PATCH 13/13] format-patch: refactor get_patch_filename Jeff King
2012-05-14 16:36         ` [PATCH 2/2] ident: report passwd errors with a more friendly message Jeff King
2012-05-10 20:04   ` [PATCH 1/2] Change error messages in ident.c Junio C Hamano
2012-05-10 20:22     ` Jeff King
2012-05-10 20:28       ` Junio C Hamano
2012-05-10 19:43 ` [PATCH 1/2] Change error messages in ident.c Make error messages caused by failed reads of the /etc/passwd file easier to understand. Signed-off-by: Angus Hammond <angusgh@gmail.com> Junio C Hamano
2012-05-10 19:57   ` Angus Hammond
2012-05-11 11:35 ` Nguyen Thai Ngoc Duy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120518232039.GI30031@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=angusgh@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.