{"thread":{"id":"30561","subject":"[PATCH 01/13] ident: split setup_ident into separate functions","startedAt":"2012-05-18T23:05:28Z","lastAt":"2013-01-25T22:10:30Z","messageCount":45,"participants":["Jeff King","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"191696","messageId":"20120518230528.GA30510@sigill.intra.peff.net","threadId":"30561","inReplyTo":"7vsjf1e2n7.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/13] ident cleanups and bugfixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:05:28Z","receivedAt":"2012-05-18T23:05:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 15, 2012 at 11:10:36AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > So it seems to me like a much simpler set of rules would be:\n> >\n> > 1. When reading gecos, always fall back to the username if the gecos\n> > field is unavailable or blank.\n> >\n> > 2. Always die when the name field is blank. That means we will die\n> > when you pass in a bogus empty GIT_COMMITTER_NAME (or an empty\n> > config name), which makes a lot more sense to me than falling back;\n> > those are bogus requests, not system config problems. And we won't\n> > ever have a blank gecos name, because we'll always fall back on the\n> > username.\n> \n> That certainly sounds very simple to explain and understand, and I do not\n> offhand think of anything *sane* that would break ;-)\n\nActually, it does end up breaking. The \"error_on_no_name\" check wants to\nsee the value _before_ the fallback, because it is not just about \"do\nnot hand out an ident with an empty name\", but rather about \"do not hand\nout this crappy fallback name when the gecos field is empty\". So we\ncannot do an \"always fallback\" behavior without breaking that check.\n\nI ended up avoiding the issue in a different way, as you'll see in patch\n8 below. Here's the series:\n\n [01/13]: ident: split setup_ident into separate functions\n [02/13]: http-push: do not access git_default_email directly\n [03/13]: fmt-merge-msg: don't use static buffer in record_person\n [04/13]: move identity config parsing to ident.c\n [05/13]: move git_default_* variables to ident.c\n [06/13]: format-patch: use default email for generating message ids\n [07/13]: fmt_ident: drop IDENT_WARN_ON_NO_NAME code\n [08/13]: ident: don't write fallback username into git_default_name\n [09/13]: drop length limitations on gecos-derived names and emails\n [10/13]: ident: report passwd errors with a more friendly message\n [11/13]: ident: use full dns names to generate email addresses\n [12/13]: ident: use a dynamic strbuf in fmt_ident\n [13/13]: format-patch: refactor get_patch_filename\n\nPatches 2, 8, and 11 fix actual bugs. The rest of it is refactoring and\ncleanup; here's the diffstat:\n\n Documentation/git-commit-tree.txt | 9 --\n Documentation/git-var.txt | 9 --\n builtin/fmt-merge-msg.c | 8 +-\n builtin/log.c | 45 ++------\n cache.h | 12 +-\n config.c | 24 +----\n environment.c | 3 -\n git-compat-util.h | 3 +\n http-push.c | 2 +-\n ident.c | 213 ++++++++++++++++---------------------\n log-tree.c | 19 ++--\n log-tree.h | 4 +-\n wrapper.c | 12 ++\n 13 files changed, 142 insertions(+), 221 deletions(-)\n\n-Peff\n"},{"id":"191687","messageId":"20120518230710.GA30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 01/13] ident: split setup_ident into separate functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:07:11Z","receivedAt":"2012-05-18T23:07:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This function sets up the default name, email, and date, and\nis not publicly available. Let's split it into three public\nfunctions so that callers can get just the parts they need.\n\nWhile we're at it, let's change the interface to simple\naccessors. The original function was called only by fmt_ident,\nand contained logic for \"if we already have some other\nvalue, don't load the default\" which properly belongs in\nfmt_ident.\n\nSigned-off-by: Jeff King \n---\nThe patch is a pain to read because it's splitting an existing function\nin 3; just reading the result is much easier.\n\n cache.h | 3 +++\n ident.c | 35 +++++++++++++++++++----------------\n 2 files changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex e14ffcd..0c095d4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -894,6 +894,9 @@ extern const char *git_author_info(int);\n extern const char *git_committer_info(int);\n extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);\n extern const char *fmt_name(const char *name, const char *email);\n+extern const char *ident_default_name(void);\n+extern const char *ident_default_email(void);\n+extern const char *ident_default_date(void);\n extern const char *git_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n \ndiff --git a/ident.c b/ident.c\nindex 87c697c..0f7dcae 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -117,21 +117,20 @@ static void copy_email(const struct passwd *pw)\n \t\t\tsizeof(git_default_email) - len);\n }\n \n-static void setup_ident(const char **name, const char **emailp)\n+const char *ident_default_name(void)\n {\n-\tstruct passwd *pw = NULL;\n-\n-\t/* Get the name (\"gecos\") */\n-\tif (!*name && !git_default_name[0]) {\n-\t\tpw = getpwuid(getuid());\n+\tif (!git_default_name[0]) {\n+\t\tstruct passwd *pw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n \t\tcopy_gecos(pw, git_default_name, sizeof(git_default_name));\n \t}\n-\tif (!*name)\n-\t\t*name = git_default_name;\n+\treturn git_default_name;\n+}\n \n-\tif (!*emailp && !git_default_email[0]) {\n+const char *ident_default_email(void)\n+{\n+\tif (!git_default_email[0]) {\n \t\tconst char *email = getenv(\"EMAIL\");\n \n \t\tif (email && email[0]) {\n@@ -139,19 +138,20 @@ static void setup_ident(const char **name, const char **emailp)\n \t\t\t\tsizeof(git_default_email));\n \t\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\t} else {\n-\t\t\tif (!pw)\n-\t\t\t\tpw = getpwuid(getuid());\n+\t\t\tstruct passwd *pw = getpwuid(getuid());\n \t\t\tif (!pw)\n \t\t\t\tdie(\"You don't exist. Go away!\");\n \t\t\tcopy_email(pw);\n \t\t}\n \t}\n-\tif (!*emailp)\n-\t\t*emailp = git_default_email;\n+\treturn git_default_email;\n+}\n \n-\t/* And set the default date */\n+const char *ident_default_date(void)\n+{\n \tif (!git_default_date[0])\n \t\tdatestamp(git_default_date, sizeof(git_default_date));\n+\treturn git_default_date;\n }\n \n static int add_raw(char *buf, size_t size, int offset, const char *str)\n@@ -311,7 +311,10 @@ const char *fmt_ident(const char *name, const char *email,\n \tint warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);\n \tint name_addr_only = (flag & IDENT_NO_DATE);\n \n-\tsetup_ident(&name, &email);\n+\tif (!name)\n+\t\tname = ident_default_name();\n+\tif (!email)\n+\t\temail = ident_default_email();\n \n \tif (!*name) {\n \t\tstruct passwd *pw;\n@@ -331,7 +334,7 @@ const char *fmt_ident(const char *name, const char *email,\n \t\tname = git_default_name;\n \t}\n \n-\tstrcpy(date, git_default_date);\n+\tstrcpy(date, ident_default_date());\n \tif (!name_addr_only && date_str && date_str[0]) {\n \t\tif (parse_date(date_str, date, sizeof(date)) < 0)\n \t\t\tdie(\"invalid date format: %s\", date_str);\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191688","messageId":"20120518230905.GB30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 02/13] http-push: do not access git_default_email directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:09:05Z","receivedAt":"2012-05-18T23:09:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"By calling the ident_default_email accessor, we can be sure\nthat the default value is actually filled-in.\n\nSigned-off-by: Jeff King \n---\nI believe this is a real, triggerable bug if you don't have your\nuser.email config set, but I didn't actually test it (and I have no idea\nif DAV would actually care about an empty email here anyway).\n\n http-push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 1df7ab5..a832ca7 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -904,7 +904,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \t\tep = strchr(ep + 1, '/');\n \t}\n \n-\tescaped = xml_entities(git_default_email);\n+\tescaped = xml_entities(ident_default_email());\n \tstrbuf_addf(&out_buffer.buf, LOCK_REQUEST, escaped);\n \tfree(escaped);\n \n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191691","messageId":"20120518231053.GC30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 03/13] fmt-merge-msg: don't use static buffer in record_person","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:10:53Z","receivedAt":"2012-05-18T23:10:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The record_person function just parses out the \"name\" field\nof the person line in a commit and adds it to a string_list.\nThe only reason we need an extra buffer is that the\nstring_list functions require a NUL-terminated string.\n\nInstead of the static buffer, we can just allocate a\ntemporary NUL-terminated copy. In addition to removing a\nuseless limit, this removes the only user of MAX_GITNAME\noutside of ident.c.\n\nSigned-off-by: Jeff King \n---\nThis actually introduces an extra malloc() per commit that we analyze. I\ndoubt we care, but if we do, the right solution IMHO is to introduce\nstring-list functions which can lookup or add a (buf, len) pair and\navoid the copy altogether.\n\n builtin/fmt-merge-msg.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex a517f17..4ee6a88 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -230,7 +230,7 @@ static void add_branch_desc(struct strbuf *out, const char *name)\n static void record_person(int which, struct string_list *people,\n \t\t\t struct commit *commit)\n {\n-\tchar name_buf[MAX_GITNAME], *name, *name_end;\n+\tchar *name_buf, *name, *name_end;\n \tstruct string_list_item *elem;\n \tconst char *field = (which == 'a') ? \"\\nauthor \" : \"\\ncommitter \";\n \n@@ -243,10 +243,9 @@ static void record_person(int which, struct string_list *people,\n \t\tname_end--;\n \twhile (isspace(*name_end) && name <= name_end)\n \t\tname_end--;\n-\tif (name_end < name || name + MAX_GITNAME <= name_end)\n+\tif (name_end < name)\n \t\treturn;\n-\tmemcpy(name_buf, name, name_end - name + 1);\n-\tname_buf[name_end - name + 1] = '\\0';\n+\tname_buf = xmemdupz(name, name_end - name + 1);\n \n \telem = string_list_lookup(people, name_buf);\n \tif (!elem) {\n@@ -254,6 +253,7 @@ static void record_person(int which, struct string_list *people,\n \t\telem->util = (void *)0;\n \t}\n \telem->util = (void*)(util_as_integral(elem) + 1);\n+\tfree(name_buf);\n }\n \n static int cmp_string_list_util_as_integral(const void *a_, const void *b_)\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191689","messageId":"20120518231103.GD30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 04/13] move identity config parsing to ident.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:11:03Z","receivedAt":"2012-05-18T23:11:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There's no reason for this to be in config, except that once\nupon a time all of the config parsing was there. It makes\nmore sense to keep the ident code together.\n\nSigned-off-by: Jeff King \n---\n cache.h | 1 +\n config.c | 24 +-----------------------\n ident.c | 21 +++++++++++++++++++++\n 3 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 0c095d4..86224c8 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -899,6 +899,7 @@ extern const char *ident_default_email(void);\n extern const char *ident_default_date(void);\n extern const char *git_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n+extern int git_ident_config(const char *, const char *, void *);\n \n struct ident_split {\n \tconst char *name_begin;\ndiff --git a/config.c b/config.c\nindex eeee986..71ef171 100644\n--- a/config.c\n+++ b/config.c\n@@ -762,28 +762,6 @@ static int git_default_core_config(const char *var, const char *value)\n \treturn 0;\n }\n \n-static int git_default_user_config(const char *var, const char *value)\n-{\n-\tif (!strcmp(var, \"user.name\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tstrlcpy(git_default_name, value, sizeof(git_default_name));\n-\t\tuser_ident_explicitly_given |= IDENT_NAME_GIVEN;\n-\t\treturn 0;\n-\t}\n-\n-\tif (!strcmp(var, \"user.email\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tstrlcpy(git_default_email, value, sizeof(git_default_email));\n-\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n-\t\treturn 0;\n-\t}\n-\n-\t/* Add other config variables here and to Documentation/config.txt. */\n-\treturn 0;\n-}\n-\n static int git_default_i18n_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"i18n.commitencoding\"))\n@@ -870,7 +848,7 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \t\treturn git_default_core_config(var, value);\n \n \tif (!prefixcmp(var, \"user.\"))\n-\t\treturn git_default_user_config(var, value);\n+\t\treturn git_ident_config(var, value, dummy);\n \n \tif (!prefixcmp(var, \"i18n.\"))\n \t\treturn git_default_i18n_config(var, value);\ndiff --git a/ident.c b/ident.c\nindex 0f7dcae..bb1158f 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -388,3 +388,24 @@ int user_ident_sufficiently_given(void)\n \treturn (user_ident_explicitly_given == IDENT_ALL_GIVEN);\n #endif\n }\n+\n+int git_ident_config(const char *var, const char *value, void *data)\n+{\n+\tif (!strcmp(var, \"user.name\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tstrlcpy(git_default_name, value, sizeof(git_default_name));\n+\t\tuser_ident_explicitly_given |= IDENT_NAME_GIVEN;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(var, \"user.email\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tstrlcpy(git_default_email, value, sizeof(git_default_email));\n+\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n+\t\treturn 0;\n+\t}\n+\n+\treturn 0;\n+}\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191690","messageId":"20120518231113.GE30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 05/13] move git_default_* variables to ident.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:11:13Z","receivedAt":"2012-05-18T23:11:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There's no reason anybody outside of ident.c should access\nthese directly (they should use the new accessors which make\nsure the variables are initialized), so we can make them\nfile-scope statics.\n\nWhile we're at it, move user_ident_explicitly_given into\nident.c; while still globally visible, it makes more sense\nto reside with the ident code.\n\nSigned-off-by: Jeff King \n---\n cache.h | 3 ---\n environment.c | 3 ---\n ident.c | 4 ++++\n 3 files changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 86224c8..f63b71f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1142,9 +1142,6 @@ struct config_include_data {\n #define CONFIG_INCLUDE_INIT { 0 }\n extern int git_config_include(const char *name, const char *value, void *data);\n \n-#define MAX_GITNAME (1000)\n-extern char git_default_email[MAX_GITNAME];\n-extern char git_default_name[MAX_GITNAME];\n #define IDENT_NAME_GIVEN 01\n #define IDENT_MAIL_GIVEN 02\n #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\ndiff --git a/environment.c b/environment.c\nindex d7e6c65..669e498 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -11,9 +11,6 @@\n #include \"refs.h\"\n #include \"fmt-merge-msg.h\"\n \n-char git_default_email[MAX_GITNAME];\n-char git_default_name[MAX_GITNAME];\n-int user_ident_explicitly_given;\n int trust_executable_bit = 1;\n int trust_ctime = 1;\n int has_symlinks = 1;\ndiff --git a/ident.c b/ident.c\nindex bb1158f..af92b2c 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -7,7 +7,11 @@\n */\n #include \"cache.h\"\n \n+#define MAX_GITNAME (1000)\n+static char git_default_name[MAX_GITNAME];\n+static char git_default_email[MAX_GITNAME];\n static char git_default_date[50];\n+int user_ident_explicitly_given;\n \n #ifdef NO_GECOS_IN_PWENT\n #define get_gecos(ignored) \"&\"\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191692","messageId":"20120518231347.GF30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 06/13] format-patch: use default email for generating message ids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:13:47Z","receivedAt":"2012-05-18T23:13:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We try to generate a sane message id for cover letters and\nthreading by appending some changing bits to the front of\nthe user's email address. The current code parses the email\nout of the results of git_committer_info, but we can do this\nmuch more easily by just calling ident_default_email\nourselves.\n\nSigned-off-by: Jeff King \n---\nTechnically this is a regression if you really wanted:\n\n GIT_COMMITTER_EMAIL=some.addr@example.com \\\n git format-patch --thread=deep\n\nto make your environment variable part of the message-ids. I don't think\nit matters, but I can adjust it if we care.\n\n builtin/log.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 690caa7..656bddf 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -737,15 +737,9 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha\n \n static void gen_message_id(struct rev_info *info, char *base)\n {\n-\tconst char *committer = git_committer_info(IDENT_WARN_ON_NO_NAME);\n-\tconst char *email_start = strrchr(committer, '<');\n-\tconst char *email_end = strrchr(committer, '>');\n \tstruct strbuf buf = STRBUF_INIT;\n-\tif (!email_start || !email_end || email_start > email_end - 1)\n-\t\tdie(_(\"Could not extract email from committer identity.\"));\n-\tstrbuf_addf(&buf, \"%s.%lu.git.%.*s\", base,\n-\t\t (unsigned long) time(NULL),\n-\t\t (int)(email_end - email_start - 1), email_start + 1);\n+\tstrbuf_addf(&buf, \"%s.%lu.git.%s\", base,\n+\t\t (unsigned long) time(NULL), ident_default_email());\n \tinfo->message_id = strbuf_detach(&buf, NULL);\n }\n \n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191693","messageId":"20120518231431.GG30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 07/13] fmt_ident: drop IDENT_WARN_ON_NO_NAME code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:14:31Z","receivedAt":"2012-05-18T23:14:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are no more callers who want this, so we can drop it.\n\nSigned-off-by: Jeff King \n---\n cache.h | 5 ++---\n ident.c | 11 ++++-------\n 2 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex f63b71f..65cbab5 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -887,9 +887,8 @@ unsigned long approxidate_careful(const char *, int *);\n unsigned long approxidate_relative(const char *date, const struct timeval *now);\n enum date_mode parse_date_format(const char *format);\n \n-#define IDENT_WARN_ON_NO_NAME 1\n-#define IDENT_ERROR_ON_NO_NAME 2\n-#define IDENT_NO_DATE\t 4\n+#define IDENT_ERROR_ON_NO_NAME 1\n+#define IDENT_NO_DATE\t 2\n extern const char *git_author_info(int);\n extern const char *git_committer_info(int);\n extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);\ndiff --git a/ident.c b/ident.c\nindex af92b2c..d2fa271 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -312,7 +312,6 @@ const char *fmt_ident(const char *name, const char *email,\n \tchar date[50];\n \tint i;\n \tint error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);\n-\tint warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);\n \tint name_addr_only = (flag & IDENT_NO_DATE);\n \n \tif (!name)\n@@ -323,13 +322,11 @@ const char *fmt_ident(const char *name, const char *email,\n \tif (!*name) {\n \t\tstruct passwd *pw;\n \n-\t\tif ((warn_on_no_name || error_on_no_name) &&\n-\t\t name == git_default_name && env_hint) {\n-\t\t\tfputs(env_hint, stderr);\n-\t\t\tenv_hint = NULL; /* warn only once */\n-\t\t}\n-\t\tif (error_on_no_name)\n+\t\tif (error_on_no_name) {\n+\t\t\tif (name == git_default_name)\n+\t\t\t\tfputs(env_hint, stderr);\n \t\t\tdie(\"empty ident %s <%s> not allowed\", name, email);\n+\t\t}\n \t\tpw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191694","messageId":"20120518231933.GH30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 08/13] ident: don't write fallback username into git_default_name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:19:33Z","receivedAt":"2012-05-18T23:19:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The fmt_ident function gets a flag that tells us whether to\ndie if the name field is blank. If it is blank and we don't\ndie, then we fall back to the username from the passwd file.\n\nThe current code writes the value into git_default_name.\nHowever, that's not necessarily correct, as the empty value\nmight have come from git_default_name, or it might have been\npassed in. This leads to two potential problems:\n\n 1. If we are overriding an empty name in the passed-in\n value, then we may be overwriting a perfectly good name\n (from gitconfig or gecos) in the git_default_name\n buffer. Later calls to fmt_ident will end up using the\n fallback name, even though a better name was available.\n\n 2. If we override an empty gecos name, we end up with the\n fallback name in git_default_name. A later call that\n uses IDENT_ERROR_ON_NO_NAME will see the fallback name\n and think that it is a good name, instead of producing\n an error. In other words, a blank gecos name would\n cause an error with this code:\n\n git_committer_info(IDENT_ERROR_ON_NO_NAME);\n\n but not this:\n\n git_committer_info(0);\n git_committer_info(IDENT_ERROR_ON_NO_NAME);\n\n because in the latter case, the first call has polluted\n the name buffer.\n\nInstead, let's make the fallback a per-invocation variable.\nWe can just use the pw->pw_name string directly, since it\nonly needs to persist through the rest of the function (and\nwe don't do any other getpwent calls).\n\nNote that while this solves (1) for future invocations of\nfmt_indent, the current invocation might use the fallback\nwhen it could in theory load a better value from\ngit_default_name. However, by not passing\nIDENT_ERROR_ON_NO_NAME, the caller is indicating that it\ndoes not care too much about the name, anyway, so we don't\nbother; this is primarily about protecting future callers\nwho do care.\n\nSigned-off-by: Jeff King \n---\nYou can trigger bug (2) by having an empty gecos field, no user.name,\nand running a merge. The recent credit_person code path will call\ngit_committer_info(0), then the commit-generating code path will call\ngit_committer_info(IDENT_ERROR_ON_NO_NAME), and you will end\nup with a commit with your username in the \"name\" field (even though the\nsecond call would want to error out).\n\nI don't think (1) is triggerable in the current code, mainly because we\ntypically are feeding getenv(\"GIT_*_NAME\") to fmt_ident, and a blank\nthere will end up causing an error later on in the program.\n\nBy the way, I left it so that:\n\n GIT_COMMITTER_NAME= git ...\n\nwill continue to fallback to the passwd username if a caller doesn't\nrequest to die on a blank name. We could pretty easily die in that case,\nbut people with flaky scripts might be affected. I figured that callers\nwho don't give ERROR_ON_NO_NAME don't really care too much what's in the\nname anyway, so there's not much point in tightening the rules.\n\n ident.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex d2fa271..f44bcb3 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -330,9 +330,7 @@ const char *fmt_ident(const char *name, const char *email,\n \t\tpw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n-\t\tstrlcpy(git_default_name, pw->pw_name,\n-\t\t\tsizeof(git_default_name));\n-\t\tname = git_default_name;\n+\t\tname = pw->pw_name;\n \t}\n \n \tstrcpy(date, ident_default_date());\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191695","messageId":"20120518232039.GI30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 09/13] drop length limitations on gecos-derived names and emails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:20:39Z","receivedAt":"2012-05-18T23:20:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we pull the user's name from the GECOS field of the\npasswd file (or generate an email address based on their\nusername and hostname), we put the result into a\nstatic buffer. While it's extremely unlikely that anybody\never hit these limits (after all, in such a case their\nparents must have hated them), we still had to deal with the\nerror cases in our code.\n\nConverting these static buffers to strbufs lets us simplify\nthe code and drop some error messages from the documentation\nthat have confused some users.\n\nThe conversion is mostly mechanical: replace string copies\nwith strbuf equivalents, and access the strbuf.buf directly.\nThere are a few exceptions:\n\n - copy_gecos and copy_email are the big winners in code\n reduction (since they no longer have to manage the\n string length manually)\n\n - git_ident_config wants to replace old versions of\n the default name (e.g., if we read the config multiple\n times), so it must reset+add to the strbuf instead of\n just adding\n\nNote that there is still one length limitation: the\ngethostname interface requires us to provide a static\nbuffer, so we arbitrarily choose 1024 bytes for the\nhostname.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-commit-tree.txt | 4 --\n Documentation/git-var.txt | 4 --\n ident.c | 100 +++++++++++++++-----------------------\n 3 files changed, 39 insertions(+), 69 deletions(-)\n\ndiff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt\nindex cfb9906..eb12b2d 100644\n--- a/Documentation/git-commit-tree.txt\n+++ b/Documentation/git-commit-tree.txt\n@@ -92,10 +92,6 @@ Diagnostics\n -----------\n You don't exist. Go away!::\n The passwd(5) gecos field couldn't be read\n-Your parents must have hated you!::\n- The passwd(5) gecos field is longer than a giant static buffer.\n-Your sysadmin must hate you!::\n- The passwd(5) name field is longer than a giant static buffer.\n \n Discussion\n ----------\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex 988a323..3f703e3 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -63,10 +63,6 @@ Diagnostics\n -----------\n You don't exist. Go away!::\n The passwd(5) gecos field couldn't be read\n-Your parents must have hated you!::\n- The passwd(5) gecos field is longer than a giant static buffer.\n-Your sysadmin must hate you!::\n- The passwd(5) name field is longer than a giant static buffer.\n \n SEE ALSO\n --------\ndiff --git a/ident.c b/ident.c\nindex f44bcb3..73a06a1 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -7,9 +7,8 @@\n */\n #include \"cache.h\"\n \n-#define MAX_GITNAME (1000)\n-static char git_default_name[MAX_GITNAME];\n-static char git_default_email[MAX_GITNAME];\n+static struct strbuf git_default_name = STRBUF_INIT;\n+static struct strbuf git_default_email = STRBUF_INIT;\n static char git_default_date[50];\n int user_ident_explicitly_given;\n \n@@ -19,42 +18,27 @@ int user_ident_explicitly_given;\n #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)\n #endif\n \n-static void copy_gecos(const struct passwd *w, char *name, size_t sz)\n+static void copy_gecos(const struct passwd *w, struct strbuf *name)\n {\n-\tchar *src, *dst;\n-\tsize_t len, nlen;\n-\n-\tnlen = strlen(w->pw_name);\n+\tchar *src;\n \n \t/* Traditionally GECOS field had office phone numbers etc, separated\n \t * with commas. Also & stands for capitalized form of the login name.\n \t */\n \n-\tfor (len = 0, dst = name, src = get_gecos(w); len < sz; src++) {\n+\tfor (src = get_gecos(w); *src && *src != ','; src++) {\n \t\tint ch = *src;\n-\t\tif (ch != '&') {\n-\t\t\t*dst++ = ch;\n-\t\t\tif (ch == 0 || ch == ',')\n-\t\t\t\tbreak;\n-\t\t\tlen++;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (len + nlen < sz) {\n+\t\tif (ch != '&')\n+\t\t\tstrbuf_addch(name, ch);\n+\t\telse {\n \t\t\t/* Sorry, Mr. McDonald... */\n-\t\t\t*dst++ = toupper(*w->pw_name);\n-\t\t\tmemcpy(dst, w->pw_name + 1, nlen - 1);\n-\t\t\tdst += nlen - 1;\n-\t\t\tlen += nlen;\n+\t\t\tstrbuf_addch(name, toupper(*w->pw_name));\n+\t\t\tstrbuf_addstr(name, w->pw_name + 1);\n \t\t}\n \t}\n-\tif (len < sz)\n-\t\tname[len] = 0;\n-\telse\n-\t\tdie(\"Your parents must have hated you!\");\n-\n }\n \n-static int add_mailname_host(char *buf, size_t len)\n+static int add_mailname_host(struct strbuf *buf)\n {\n \tFILE *mailname;\n \n@@ -65,7 +49,7 @@ static int add_mailname_host(char *buf, size_t len)\n \t\t\t\tstrerror(errno));\n \t\treturn -1;\n \t}\n-\tif (!fgets(buf, len, mailname)) {\n+\tif (strbuf_getline(buf, mailname, '\\n') == EOF) {\n \t\tif (ferror(mailname))\n \t\t\twarning(\"cannot read /etc/mailname: %s\",\n \t\t\t\tstrerror(errno));\n@@ -77,78 +61,70 @@ static int add_mailname_host(char *buf, size_t len)\n \treturn 0;\n }\n \n-static void add_domainname(char *buf, size_t len)\n+static void add_domainname(struct strbuf *out)\n {\n+\tchar buf[1024];\n \tstruct hostent *he;\n-\tsize_t namelen;\n \tconst char *domainname;\n \n-\tif (gethostname(buf, len)) {\n+\tif (gethostname(buf, sizeof(buf))) {\n \t\twarning(\"cannot get host name: %s\", strerror(errno));\n-\t\tstrlcpy(buf, \"(none)\", len);\n+\t\tstrbuf_addstr(out, \"(none)\");\n \t\treturn;\n \t}\n-\tnamelen = strlen(buf);\n-\tif (memchr(buf, '.', namelen))\n+\tstrbuf_addstr(out, buf);\n+\tif (strchr(buf, '.'))\n \t\treturn;\n \n \the = gethostbyname(buf);\n-\tbuf[namelen++] = '.';\n-\tbuf += namelen;\n-\tlen -= namelen;\n+\tstrbuf_addch(out, '.');\n \tif (he && (domainname = strchr(he->h_name, '.')))\n-\t\tstrlcpy(buf, domainname + 1, len);\n+\t\tstrbuf_addstr(out, domainname + 1);\n \telse\n-\t\tstrlcpy(buf, \"(none)\", len);\n+\t\tstrbuf_addstr(out, \"(none)\");\n }\n \n-static void copy_email(const struct passwd *pw)\n+static void copy_email(const struct passwd *pw, struct strbuf *email)\n {\n \t/*\n \t * Make up a fake email address\n \t * (name + '@' + hostname [+ '.' + domainname])\n \t */\n-\tsize_t len = strlen(pw->pw_name);\n-\tif (len > sizeof(git_default_email)/2)\n-\t\tdie(\"Your sysadmin must hate you!\");\n-\tmemcpy(git_default_email, pw->pw_name, len);\n-\tgit_default_email[len++] = '@';\n-\n-\tif (!add_mailname_host(git_default_email + len,\n-\t\t\t\tsizeof(git_default_email) - len))\n+\tstrbuf_addstr(email, pw->pw_name);\n+\tstrbuf_addch(email, '@');\n+\n+\tif (!add_mailname_host(email))\n \t\treturn;\t/* read from \"/etc/mailname\" (Debian) */\n-\tadd_domainname(git_default_email + len,\n-\t\t\tsizeof(git_default_email) - len);\n+\tadd_domainname(email);\n }\n \n const char *ident_default_name(void)\n {\n-\tif (!git_default_name[0]) {\n+\tif (!git_default_name.len) {\n \t\tstruct passwd *pw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n-\t\tcopy_gecos(pw, git_default_name, sizeof(git_default_name));\n+\t\tcopy_gecos(pw, &git_default_name);\n \t}\n-\treturn git_default_name;\n+\treturn git_default_name.buf;\n }\n \n const char *ident_default_email(void)\n {\n-\tif (!git_default_email[0]) {\n+\tif (!git_default_email.len) {\n \t\tconst char *email = getenv(\"EMAIL\");\n \n \t\tif (email && email[0]) {\n-\t\t\tstrlcpy(git_default_email, email,\n-\t\t\t\tsizeof(git_default_email));\n+\t\t\tstrbuf_addstr(&git_default_email, email);\n \t\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\t} else {\n \t\t\tstruct passwd *pw = getpwuid(getuid());\n \t\t\tif (!pw)\n \t\t\t\tdie(\"You don't exist. Go away!\");\n-\t\t\tcopy_email(pw);\n+\t\t\tcopy_email(pw, &git_default_email);\n \t\t}\n \t}\n-\treturn git_default_email;\n+\treturn git_default_email.buf;\n }\n \n const char *ident_default_date(void)\n@@ -323,7 +299,7 @@ const char *fmt_ident(const char *name, const char *email,\n \t\tstruct passwd *pw;\n \n \t\tif (error_on_no_name) {\n-\t\t\tif (name == git_default_name)\n+\t\t\tif (name == git_default_name.buf)\n \t\t\t\tfputs(env_hint, stderr);\n \t\t\tdie(\"empty ident %s <%s> not allowed\", name, email);\n \t\t}\n@@ -393,7 +369,8 @@ int git_ident_config(const char *var, const char *value, void *data)\n \tif (!strcmp(var, \"user.name\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tstrlcpy(git_default_name, value, sizeof(git_default_name));\n+\t\tstrbuf_reset(&git_default_name);\n+\t\tstrbuf_addstr(&git_default_name, value);\n \t\tuser_ident_explicitly_given |= IDENT_NAME_GIVEN;\n \t\treturn 0;\n \t}\n@@ -401,7 +378,8 @@ int git_ident_config(const char *var, const char *value, void *data)\n \tif (!strcmp(var, \"user.email\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tstrlcpy(git_default_email, value, sizeof(git_default_email));\n+\t\tstrbuf_reset(&git_default_email);\n+\t\tstrbuf_addstr(&git_default_email, value);\n \t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\treturn 0;\n \t}\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191697","messageId":"20120518232106.GJ30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 10/13] ident: report passwd errors with a more friendly message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:21:07Z","receivedAt":"2012-05-18T23:21:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When getpwuid fails, we give a cute but cryptic message.\nWhile it makes sense if you know that getpwuid or identity\nfunctions are being called, this code is triggered behind\nthe scenes by quite a few git commands these days (e.g.,\nreceive-pack on a remote server might use it for a reflog;\nthe current message is hard to distinguish from an\nauthentication error). Let's switch to something that gives\na little more context.\n\nWhile we're at it, we can factor out all of the\ncut-and-pastes of the \"you don't exist\" message into a\nwrapper function. Rather than provide xgetpwuid, let's make\nit even more specific to just getting the passwd entry for\nthe current uid. That's the only way we use getpwuid anyway,\nand it lets us make an even more specific error message.\n\nThe current message also fails to mention errno. While the\nusual cause for getpwuid failing is that the user does not\nexist, mentioning errno makes it easier to diagnose these\nproblems. Note that POSIX specifies that errno remain\nuntouched if the passwd entry does not exist (but will be\nset on actual errors), whereas some systems will return\nENOENT or similar for a missing entry. We handle both cases\nin our wrapper.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-commit-tree.txt | 5 -----\n Documentation/git-var.txt | 5 -----\n git-compat-util.h | 3 +++\n ident.c | 20 +++++---------------\n wrapper.c | 12 ++++++++++++\n 5 files changed, 20 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt\nindex eb12b2d..eb8ee99 100644\n--- a/Documentation/git-commit-tree.txt\n+++ b/Documentation/git-commit-tree.txt\n@@ -88,11 +88,6 @@ for one to be entered and terminated with ^D.\n \n include::date-formats.txt[]\n \n-Diagnostics\n------------\n-You don't exist. Go away!::\n- The passwd(5) gecos field couldn't be read\n-\n Discussion\n ----------\n \ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex 3f703e3..67edf58 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -59,11 +59,6 @@ ifdef::git-default-pager[]\n The build you are using chose '{git-default-pager}' as the default.\n endif::git-default-pager[]\n \n-Diagnostics\n------------\n-You don't exist. Go away!::\n- The passwd(5) gecos field couldn't be read\n-\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ed11ad8..5bd9ad7 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -595,4 +595,7 @@ int rmdir_or_warn(const char *path);\n */\n int remove_or_warn(unsigned int mode, const char *path);\n \n+/* Get the passwd entry for the UID of the current process. */\n+struct passwd *xgetpwuid_self(void);\n+\n #endif\ndiff --git a/ident.c b/ident.c\nindex 73a06a1..5aec073 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -100,12 +100,8 @@ static void copy_email(const struct passwd *pw, struct strbuf *email)\n \n const char *ident_default_name(void)\n {\n-\tif (!git_default_name.len) {\n-\t\tstruct passwd *pw = getpwuid(getuid());\n-\t\tif (!pw)\n-\t\t\tdie(\"You don't exist. Go away!\");\n-\t\tcopy_gecos(pw, &git_default_name);\n-\t}\n+\tif (!git_default_name.len)\n+\t\tcopy_gecos(xgetpwuid_self(), &git_default_name);\n \treturn git_default_name.buf;\n }\n \n@@ -117,12 +113,8 @@ const char *ident_default_email(void)\n \t\tif (email && email[0]) {\n \t\t\tstrbuf_addstr(&git_default_email, email);\n \t\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n-\t\t} else {\n-\t\t\tstruct passwd *pw = getpwuid(getuid());\n-\t\t\tif (!pw)\n-\t\t\t\tdie(\"You don't exist. Go away!\");\n-\t\t\tcopy_email(pw, &git_default_email);\n-\t\t}\n+\t\t} else\n+\t\t\tcopy_email(xgetpwuid_self(), &git_default_email);\n \t}\n \treturn git_default_email.buf;\n }\n@@ -303,9 +295,7 @@ const char *fmt_ident(const char *name, const char *email,\n \t\t\t\tfputs(env_hint, stderr);\n \t\t\tdie(\"empty ident %s <%s> not allowed\", name, email);\n \t\t}\n-\t\tpw = getpwuid(getuid());\n-\t\tif (!pw)\n-\t\t\tdie(\"You don't exist. Go away!\");\n+\t\tpw = xgetpwuid_self();\n \t\tname = pw->pw_name;\n \t}\n \ndiff --git a/wrapper.c b/wrapper.c\nindex 6ccd059..b5e33e4 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -402,3 +402,15 @@ int remove_or_warn(unsigned int mode, const char *file)\n {\n \treturn S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file);\n }\n+\n+struct passwd *xgetpwuid_self(void)\n+{\n+\tstruct passwd *pw;\n+\n+\terrno = 0;\n+\tpw = getpwuid(getuid());\n+\tif (!pw)\n+\t\tdie(_(\"unable to look up current user in the passwd file: %s\"),\n+\t\t errno ? strerror(errno) : _(\"no such user\"));\n+\treturn pw;\n+}\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191698","messageId":"20120518232256.GK30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 11/13] ident: use full dns names to generate email addresses","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:22:56Z","receivedAt":"2012-05-18T23:22:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we construct an email address from the username and\nhostname, we generate the host part of the email with this\nprocedure:\n\n 1. add the result of gethostname\n\n 2. if it has a dot, ok, it's fully qualified\n\n 3. if not, then look up the unqualified hostname via\n gethostbyname; take the domain name of the result and\n append it to the hostname\n\nStep 3 can actually produce a bogus result, as the name\nreturned by gethostbyname may not be related to the hostname\nwe fed it (e.g., consider a machine \"foo\" with names\n\"foo.one.example.com\" and \"bar.two.example.com\"; we may have\nthe latter returned and generate the bogus name\n\"foo.two.example.com\").\n\nThis patch simply uses the full hostname returned by\ngethostbyname. In the common case that the first part is the\nsame as the unqualified hostname, the behavior is identical.\nAnd in the case that it is not the same, we are much more\nlikely to be generating a valid name.\n\nSigned-off-by: Jeff King \n---\nIt takes a pretty odd /etc/hosts setup, but I was able to trigger this\nbug. I doubt anyone is affected in practice, but hey, the right code is\n5 lines shorter (and easier to understand).\n\n ident.c | 13 ++++---------\n 1 file changed, 4 insertions(+), 9 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 5aec073..b111e34 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -65,23 +65,18 @@ static void add_domainname(struct strbuf *out)\n {\n \tchar buf[1024];\n \tstruct hostent *he;\n-\tconst char *domainname;\n \n \tif (gethostname(buf, sizeof(buf))) {\n \t\twarning(\"cannot get host name: %s\", strerror(errno));\n \t\tstrbuf_addstr(out, \"(none)\");\n \t\treturn;\n \t}\n-\tstrbuf_addstr(out, buf);\n \tif (strchr(buf, '.'))\n-\t\treturn;\n-\n-\the = gethostbyname(buf);\n-\tstrbuf_addch(out, '.');\n-\tif (he && (domainname = strchr(he->h_name, '.')))\n-\t\tstrbuf_addstr(out, domainname + 1);\n+\t\tstrbuf_addstr(out, buf);\n+\telse if ((he = gethostbyname(buf)) && strchr(he->h_name, '.'))\n+\t\tstrbuf_addstr(out, he->h_name);\n \telse\n-\t\tstrbuf_addstr(out, \"(none)\");\n+\t\tstrbuf_addf(out, \"%s.(none)\", buf);\n }\n \n static void copy_email(const struct passwd *pw, struct strbuf *email)\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191699","messageId":"20120518232338.GL30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 12/13] ident: use a dynamic strbuf in fmt_ident","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:23:39Z","receivedAt":"2012-05-18T23:23:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that we accept arbitrary-sized names and email\naddresses, the only remaining limit is in the actual\nformatting of the names into a buffer. The current limit is\n1000 characters, which is not likely to be reached, but\nusing a strbuf is one less error condition we have to worry\nabout.\n\nSigned-off-by: Jeff King \n---\n ident.c | 43 +++++++++++++++----------------------------\n 1 file changed, 15 insertions(+), 28 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex b111e34..cefb829 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -121,15 +121,6 @@ const char *ident_default_date(void)\n \treturn git_default_date;\n }\n \n-static int add_raw(char *buf, size_t size, int offset, const char *str)\n-{\n-\tsize_t len = strlen(str);\n-\tif (offset + len > size)\n-\t\treturn size;\n-\tmemcpy(buf + offset, str, len);\n-\treturn offset + len;\n-}\n-\n static int crud(unsigned char c)\n {\n \treturn c <= 32 ||\n@@ -148,7 +139,7 @@ static int crud(unsigned char c)\n * Copy over a string to the destination, but avoid special\n * characters ('\\n', '<' and '>') and remove crud at the end\n */\n-static int copy(char *buf, size_t size, int offset, const char *src)\n+static void strbuf_addstr_without_crud(struct strbuf *sb, const char *src)\n {\n \tsize_t i, len;\n \tunsigned char c;\n@@ -172,19 +163,19 @@ static int copy(char *buf, size_t size, int offset, const char *src)\n \t/*\n \t * Copy the rest to the buffer, but avoid the special\n \t * characters '\\n' '<' and '>' that act as delimiters on\n-\t * an identification line\n+\t * an identification line. We can only remove crud, never add it,\n+\t * so 'len' is our maximum.\n \t */\n+\tstrbuf_grow(sb, len);\n \tfor (i = 0; i < len; i++) {\n \t\tc = *src++;\n \t\tswitch (c) {\n \t\tcase '\\n': case '<': case '>':\n \t\t\tcontinue;\n \t\t}\n-\t\tif (offset >= size)\n-\t\t\treturn size;\n-\t\tbuf[offset++] = c;\n+\t\tsb->buf[sb->len++] = c;\n \t}\n-\treturn offset;\n+\tsb->buf[sb->len] = '\\0';\n }\n \n /*\n@@ -271,9 +262,8 @@ static const char *env_hint =\n const char *fmt_ident(const char *name, const char *email,\n \t\t const char *date_str, int flag)\n {\n-\tstatic char buffer[1000];\n+\tstatic struct strbuf ident = STRBUF_INIT;\n \tchar date[50];\n-\tint i;\n \tint error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);\n \tint name_addr_only = (flag & IDENT_NO_DATE);\n \n@@ -300,19 +290,16 @@ const char *fmt_ident(const char *name, const char *email,\n \t\t\tdie(\"invalid date format: %s\", date_str);\n \t}\n \n-\ti = copy(buffer, sizeof(buffer), 0, name);\n-\ti = add_raw(buffer, sizeof(buffer), i, \" <\");\n-\ti = copy(buffer, sizeof(buffer), i, email);\n+\tstrbuf_reset(&ident);\n+\tstrbuf_addstr_without_crud(&ident, name);\n+\tstrbuf_addstr(&ident, \" <\");\n+\tstrbuf_addstr_without_crud(&ident, email);\n+\tstrbuf_addch(&ident, '>');\n \tif (!name_addr_only) {\n-\t\ti = add_raw(buffer, sizeof(buffer), i, \"> \");\n-\t\ti = copy(buffer, sizeof(buffer), i, date);\n-\t} else {\n-\t\ti = add_raw(buffer, sizeof(buffer), i, \">\");\n+\t\tstrbuf_addch(&ident, ' ');\n+\t\tstrbuf_addstr_without_crud(&ident, date);\n \t}\n-\tif (i >= sizeof(buffer))\n-\t\tdie(\"Impossibly long personal identifier\");\n-\tbuffer[i] = 0;\n-\treturn buffer;\n+\treturn ident.buf;\n }\n \n const char *fmt_name(const char *name, const char *email)\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191700","messageId":"20120518232449.GM30031@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120518230528.GA30510@sigill.intra.peff.net","subject":"[PATCH 13/13] format-patch: refactor get_patch_filename","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-18T23:24:49Z","receivedAt":"2012-05-18T23:24:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The get_patch_filename function expects a commit argument\nand uses it to get the sanitized subject line when making a\npatch filename. However, we also want to use this same\nfunction for the cover letter, which does not have a commit\nobject. The current solution is to create a fake commit with\nthe subject \"cover letter\". Instead, let's make the\nget_patch_filename interface more flexibile, and allow\npassing a direct subject.\n\nSigned-off-by: Jeff King \n---\nThis one is a bonus. It doesn't really have anything to do with ident,\nexcept that I came across it while trying to figure out how the\ncommitter info is being used in make_cover_letter.\n\n builtin/log.c | 35 +++++++----------------------------\n log-tree.c | 19 +++++++++++--------\n log-tree.h | 4 ++--\n 3 files changed, 20 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 656bddf..8010a40 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -663,7 +663,8 @@ static FILE *realstdout = NULL;\n static const char *output_directory = NULL;\n static int outdir_offset;\n \n-static int reopen_stdout(struct commit *commit, struct rev_info *rev, int quiet)\n+static int reopen_stdout(struct commit *commit, const char *subject,\n+\t\t\t struct rev_info *rev, int quiet)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n@@ -677,7 +678,7 @@ static int reopen_stdout(struct commit *commit, struct rev_info *rev, int quiet)\n \t\t\tstrbuf_addch(&filename, '/');\n \t}\n \n-\tget_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);\n+\tget_patch_filename(commit, subject, rev->nr, fmt_patch_suffix, &filename);\n \n \tif (!quiet)\n \t\tfprintf(realstdout, \"%s\\n\", filename.buf + outdir_offset);\n@@ -778,7 +779,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tconst char *encoding = \"UTF-8\";\n \tstruct diff_options opts;\n \tint need_8bit_cte = 0;\n-\tstruct commit *commit = NULL;\n \tstruct pretty_print_context pp = {0};\n \n \tif (rev->commit_format != CMIT_FMT_EMAIL)\n@@ -786,31 +786,10 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \n \tcommitter = git_committer_info(0);\n \n-\tif (!numbered_files) {\n-\t\t/*\n-\t\t * We fake a commit for the cover letter so we get the filename\n-\t\t * desired.\n-\t\t */\n-\t\tcommit = xcalloc(1, sizeof(*commit));\n-\t\tcommit->buffer = xmalloc(400);\n-\t\tsnprintf(commit->buffer, 400,\n-\t\t\t\"tree 0000000000000000000000000000000000000000\\n\"\n-\t\t\t\"parent %s\\n\"\n-\t\t\t\"author %s\\n\"\n-\t\t\t\"committer %s\\n\\n\"\n-\t\t\t\"cover letter\\n\",\n-\t\t\tsha1_to_hex(head->object.sha1), committer, committer);\n-\t}\n-\n-\tif (!use_stdout && reopen_stdout(commit, rev, quiet))\n+\tif (!use_stdout &&\n+\t reopen_stdout(NULL, numbered_files ? NULL : \"cover-letter\", rev, quiet))\n \t\treturn;\n \n-\tif (commit) {\n-\n-\t\tfree(commit->buffer);\n-\t\tfree(commit);\n-\t}\n-\n \tlog_write_email_headers(rev, head, &pp.subject, &pp.after_subject,\n \t\t\t\t&need_8bit_cte);\n \n@@ -1405,8 +1384,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tgen_message_id(&rev, sha1_to_hex(commit->object.sha1));\n \t\t}\n \n-\t\tif (!use_stdout && reopen_stdout(numbered_files ? NULL : commit,\n-\t\t\t\t\t\t &rev, quiet))\n+\t\tif (!use_stdout &&\n+\t\t reopen_stdout(numbered_files ? NULL : commit, NULL, &rev, quiet))\n \t\t\tdie(_(\"Failed to create output files\"));\n \t\tshown = log_tree_commit(&rev, commit);\n \t\tfree(commit->buffer);\ndiff --git a/log-tree.c b/log-tree.c\nindex 376d973..c894930 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -299,19 +299,22 @@ static unsigned int digits_in_number(unsigned int number)\n \treturn result;\n }\n \n-void get_patch_filename(struct commit *commit, int nr, const char *suffix,\n-\t\t\tstruct strbuf *buf)\n+void get_patch_filename(struct commit *commit, const char *subject, int nr,\n+\t\t\tconst char *suffix, struct strbuf *buf)\n {\n \tint suffix_len = strlen(suffix) + 1;\n \tint start_len = buf->len;\n \n-\tstrbuf_addf(buf, commit ? \"%04d-\" : \"%d\", nr);\n-\tif (commit) {\n+\tstrbuf_addf(buf, commit || subject ? \"%04d-\" : \"%d\", nr);\n+\tif (commit || subject) {\n \t\tint max_len = start_len + FORMAT_PATCH_NAME_MAX - suffix_len;\n \t\tstruct pretty_print_context ctx = {0};\n-\t\tctx.date_mode = DATE_NORMAL;\n \n-\t\tformat_commit_message(commit, \"%f\", buf, &ctx);\n+\t\tif (subject)\n+\t\t\tstrbuf_addstr(buf, subject);\n+\t\telse if (commit)\n+\t\t\tformat_commit_message(commit, \"%f\", buf, &ctx);\n+\n \t\tif (max_len < buf->len)\n \t\t\tstrbuf_setlen(buf, max_len);\n \t\tstrbuf_addstr(buf, suffix);\n@@ -384,8 +387,8 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t mime_boundary_leader, opt->mime_boundary);\n \t\textra_headers = subject_buffer;\n \n-\t\tget_patch_filename(opt->numbered_files ? NULL : commit, opt->nr,\n-\t\t\t\t opt->patch_suffix, &filename);\n+\t\tget_patch_filename(opt->numbered_files ? NULL : commit, NULL,\n+\t\t\t\t opt->nr, opt->patch_suffix, &filename);\n \t\tsnprintf(buffer, sizeof(buffer) - 1,\n \t\t\t \"\\n--%s%s\\n\"\n \t\t\t \"Content-Type: text/x-patch;\"\ndiff --git a/log-tree.h b/log-tree.h\nindex 5c4cf7c..f5ac238 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -21,7 +21,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n void load_ref_decorations(int flags);\n \n #define FORMAT_PATCH_NAME_MAX 64\n-void get_patch_filename(struct commit *commit, int nr, const char *suffix,\n-\t\t\tstruct strbuf *buf);\n+void get_patch_filename(struct commit *commit, const char *subject, int nr,\n+\t\t\tconst char *suffix, struct strbuf *buf);\n \n #endif\n-- \n1.7.10.1.16.g53a707b\n"},{"id":"191779","messageId":"7v7gw69rbz.fsf@alter.siamese.dyndns.org","threadId":"30561","inReplyTo":"20120518231933.GH30031@sigill.intra.peff.net","subject":"Re: [PATCH 08/13] ident: don't write fallback username into git_default_name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-21T02:54:24Z","receivedAt":"2012-05-21T02:54:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This step seems to break t/t4014-format-patch.sh; the last output when the\ntest is run with \"-i -v\" indicates that Message-Id: and In-Reply-To: lines\nhave \"\" on it.\n"},{"id":"191780","messageId":"7vzk928clf.fsf@alter.siamese.dyndns.org","threadId":"30561","inReplyTo":"20120518231347.GF30031@sigill.intra.peff.net","subject":"Re: [PATCH 06/13] format-patch: use default email for generating message ids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-21T02:58:04Z","receivedAt":"2012-05-21T02:58:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> We try to generate a sane message id for cover letters and\n> threading by appending some changing bits to the front of\n> the user's email address. The current code parses the email\n> out of the results of git_committer_info, but we can do this\n> much more easily by just calling ident_default_email\n> ourselves.\n>\n> Signed-off-by: Jeff King \n> ---\n> Technically this is a regression if you really wanted:\n>\n> GIT_COMMITTER_EMAIL=some.addr@example.com \\\n> git format-patch --thread=deep\n>\n> to make your environment variable part of the message-ids. I don't think\n> it matters, but I can adjust it if we care.\n\nIs it because you no longer explicitly ask for \"committer\" and get generic\n\"who am I\" bit from the ident infrastructure?\n\nI wouldn't be surprised if some automated \"commit email notification\nreacting to push\" bot in post-receive hook is using the environment\nvariable to affect the message ID. I would doubt that would break the\nmessage as long as the message ID generated from this codepath stays\nvalid, though, so I wouldn't worry about complaints along the lines of\n\"you started using names different from what you used to use\". As long as\nwe don't die due to \"Hey bot, you do not seem to have a valid e-mail\naddress!\", I don't think we need to worry about it.\n"},{"id":"191781","messageId":"7vsjeu89dd.fsf@alter.siamese.dyndns.org","threadId":"30561","inReplyTo":"20120518231113.GE30031@sigill.intra.peff.net","subject":"Re: [PATCH 05/13] move git_default_* variables to ident.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-21T04:07:42Z","receivedAt":"2012-05-21T04:07:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> diff --git a/cache.h b/cache.h\n> index 86224c8..f63b71f 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1142,9 +1142,6 @@ struct config_include_data {\n> #define CONFIG_INCLUDE_INIT { 0 }\n> extern int git_config_include(const char *name, const char *value, void *data);\n> \n> -#define MAX_GITNAME (1000)\n> -extern char git_default_email[MAX_GITNAME];\n> -extern char git_default_name[MAX_GITNAME];\n> #define IDENT_NAME_GIVEN 01\n> #define IDENT_MAIL_GIVEN 02\n> #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\n\nDuring this step, builtin/fmt-merge-msg.c ceases to compile, even though\nin the end it is fixed...\n"},{"id":"191783","messageId":"20120521054118.GA2077@sigill.intra.peff.net","threadId":"30561","inReplyTo":"7vsjeu89dd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 05/13] move git_default_* variables to ident.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T05:41:18Z","receivedAt":"2012-05-21T05:41:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, May 20, 2012 at 09:07:42PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > diff --git a/cache.h b/cache.h\n> > index 86224c8..f63b71f 100644\n> > --- a/cache.h\n> > +++ b/cache.h\n> > @@ -1142,9 +1142,6 @@ struct config_include_data {\n> > #define CONFIG_INCLUDE_INIT { 0 }\n> > extern int git_config_include(const char *name, const char *value, void *data);\n> > \n> > -#define MAX_GITNAME (1000)\n> > -extern char git_default_email[MAX_GITNAME];\n> > -extern char git_default_name[MAX_GITNAME];\n> > #define IDENT_NAME_GIVEN 01\n> > #define IDENT_MAIL_GIVEN 02\n> > #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\n> \n> During this step, builtin/fmt-merge-msg.c ceases to compile, even though\n> in the end it is fixed...\n\nHrm. I ran the series through \"make\" on each commit before submitting,\nand it compiled fine. I just did it again to double-check, and it's\nstill fine.\n\nWhat was the error you got? Are you sure you had 03/13 in place\nbeforehand, which removes the use of MAX_GITNAME from\nbuiltin/fmt-merge-msg.c?\n\n-Peff\n"},{"id":"191784","messageId":"20120521063145.GB2077@sigill.intra.peff.net","threadId":"30561","inReplyTo":"7v7gw69rbz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 08/13] ident: don't write fallback username into git_default_name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T06:31:45Z","receivedAt":"2012-05-21T06:31:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, May 20, 2012 at 07:54:24PM -0700, Junio C Hamano wrote:\n\n> This step seems to break t/t4014-format-patch.sh; the last output when the\n> test is run with \"-i -v\" indicates that Message-Id: and In-Reply-To: lines\n> have \" \">\" on it.\n\nI found the issue. The test failure is actually introduced by patch\n06/13 (format-patch: use default email for generating message ids). But\nthen it later gets fixed in 09/13 (drop length limitations on\ngecos-derived names and emails).\n\nThe problems turns out to be a latent bug hidden in add_mailname_host.\nIt uses fgets to read from /etc/mailname, but never trims the trailing\nnewline. So git_default_email ends up set to something like\n\"peff@peff.net\\n\". Most of the time, this bug gets covered up by the\nnormalization that happens in fmt_ident (where we remove cruft\ncharacters like newline). But in patch 6, we start using it directly\ninstead of re-parsing the output of fmt_ident, introducing a noticeable\nbug.\n\nI suspect there is also a bug in http-push.c, which directly uses\ngit_default_email as part of the xml in the dav request. I don't know if\nit actually matters there or not. And of course this triggers only on\nsystems with /etc/mailname, and only when you don't have user.email set.\nSo it may just be that the combination of that and the fact that hardly\nanybody uses dumb http push has caused it to remain hidden.\n\nThe bug later ends up fixed in patch 9, because we replace fgets with\nstrbuf_getline, which trims the newline for us. Placing the patch below\nanywhere before patch 6 fixes the issue (and then patch 9 would need to\nlater remove the code).\n\nIt does raise a subtle issue, though: should we be trimming whitespace\nand other undesirable characters from git_default_email (or\ngit_default_name)? We don't currently, and it ends up OK because the\nresult typically is fed through fmt_ident, which cleans it up. But:\n\n 1. We do look at the git_default_* variables for things like deciding\n whether the name is blank. So if your gecos field was \" \", I think\n that would fool git into thinking it had something useful, and skip\n the IDENT_ERROR_ON_NO_NAME check, even though fmt_ident would\n produce an empty name.\n\n 2. We don't always feed it through fmt_ident (the http-push.c callsite\n I mentioned above, and now patch 6 adds another one).\n\nSo I think my preference would be:\n\n - apply the patch below as 5.5/13\n\n - tweak patch 9 to remove the extra trimming\n\n - add a patch 14 to call strbuf_trim on the name and email buffers\n after reading them from system files.\n\n-- >8 --\nSubject: [PATCH] ident: trim trailing whitespace from /etc/mailname\n\nOtherwise we will typically end up with an extra newline in\nour git_default_email. Most of the time this doesn't matter,\nas fmt_ident will skip it as cruft, but there is one code\npath that accesses it directly (in http-push.c:lock_remote).\n\nLet's trim any trailing space or line termination when we\nread from /etc/mailname.\n\nSigned-off-by: Jeff King \n---\n ident.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/ident.c b/ident.c\nindex af92b2c..e797e36 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -57,6 +57,7 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)\n static int add_mailname_host(char *buf, size_t len)\n {\n \tFILE *mailname;\n+\tchar *p;\n \n \tmailname = fopen(\"/etc/mailname\", \"r\");\n \tif (!mailname) {\n@@ -74,6 +75,8 @@ static int add_mailname_host(char *buf, size_t len)\n \t}\n \t/* success! */\n \tfclose(mailname);\n+\tfor (p = buf + strlen(buf) - 1; p >= buf && isspace(*p); p--)\n+\t\t*p = '\\0';\n \treturn 0;\n }\n \n-- \n1.7.10.1.19.g711d603\n"},{"id":"191785","messageId":"20120521063643.GC2077@sigill.intra.peff.net","threadId":"30561","inReplyTo":"7vzk928clf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 06/13] format-patch: use default email for generating message ids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T06:36:43Z","receivedAt":"2012-05-21T06:36:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, May 20, 2012 at 07:58:04PM -0700, Junio C Hamano wrote:\n\n> > Technically this is a regression if you really wanted:\n> >\n> > GIT_COMMITTER_EMAIL=some.addr@example.com \\\n> > git format-patch --thread=deep\n> >\n> > to make your environment variable part of the message-ids. I don't think\n> > it matters, but I can adjust it if we care.\n> \n> Is it because you no longer explicitly ask for \"committer\" and get generic\n> \"who am I\" bit from the ident infrastructure?\n\nRight. Calling git_committer_info will also check\ngetenv(\"GIT_COMMITTER_EMAIL\"), but we don't bother to do that here. We\ncould change the code to:\n\n const char *email = getenv(\"GIT_COMMITTER_EMAIL\");\n if (!email)\n email = ident_default_email();\n\nif it matters. I don't think it's worth building an alternate version of\ngit_committer_info that doesn't do the \"name\" half of the info.\n\n> I wouldn't be surprised if some automated \"commit email notification\n> reacting to push\" bot in post-receive hook is using the environment\n> variable to affect the message ID. I would doubt that would break the\n> message as long as the message ID generated from this codepath stays\n> valid, though, so I wouldn't worry about complaints along the lines of\n> \"you started using names different from what you used to use\". As long as\n> we don't die due to \"Hey bot, you do not seem to have a valid e-mail\n> address!\", I don't think we need to worry about it.\n\nNo, we'll never die as a result. But if you have a poorly configured\nmachine, you might get \"user@host.(none)\". Or I suppose you could leak\ninformation about the username of the post-receive process. In both\ncases, we do this already, so the only change is if you are trying to\noverride that with GIT_COMMITTER_EMAIL.\n\nSo it's a little far-fetched, which is why I didn't bother. But the code\nabove is quite simple, so maybe it is better to be conservative.\n\n-Peff\n"},{"id":"191786","messageId":"20120521064117.GA21472@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521054118.GA2077@sigill.intra.peff.net","subject":"Re: [PATCH 05/13] move git_default_* variables to ident.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T06:41:18Z","receivedAt":"2012-05-21T06:41:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 21, 2012 at 01:41:18AM -0400, Jeff King wrote:\n\n> > > -#define MAX_GITNAME (1000)\n> > > -extern char git_default_email[MAX_GITNAME];\n> > > -extern char git_default_name[MAX_GITNAME];\n> > > #define IDENT_NAME_GIVEN 01\n> > > #define IDENT_MAIL_GIVEN 02\n> > > #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\n> > \n> > During this step, builtin/fmt-merge-msg.c ceases to compile, even though\n> > in the end it is fixed...\n> \n> Hrm. I ran the series through \"make\" on each commit before submitting,\n> and it compiled fine. I just did it again to double-check, and it's\n> still fine.\n> \n> What was the error you got? Are you sure you had 03/13 in place\n> beforehand, which removes the use of MAX_GITNAME from\n> builtin/fmt-merge-msg.c?\n\nAh, yeah. I just looked at what you wrote in the \"What's Cooking\"\nmessage. For some reason you applied patch 03/13 out of order (in slot\n5). I'm not sure how you order the messages in a series when you apply\nit, but AFAICT, the numbers in the subjects and the date stamps are\nin the right order. Operator error? :)\n\n-Peff\n"},{"id":"191797","messageId":"7v396t99up.fsf@alter.siamese.dyndns.org","threadId":"30561","inReplyTo":"20120521063145.GB2077@sigill.intra.peff.net","subject":"Re: [PATCH 08/13] ident: don't write fallback username into git_default_name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-21T09:11:58Z","receivedAt":"2012-05-21T09:11:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It does raise a subtle issue, though: should we be trimming whitespace\n> and other undesirable characters from git_default_email (or\n> git_default_name)? We don't currently, and it ends up OK because the\n> result typically is fed through fmt_ident, which cleans it up. But:\n>\n> 1. We do look at the git_default_* variables for things like deciding\n> whether the name is blank. So if your gecos field was \" \", I think\n> that would fool git into thinking it had something useful, and skip\n> the IDENT_ERROR_ON_NO_NAME check, even though fmt_ident would\n> produce an empty name.\n>\n> 2. We don't always feed it through fmt_ident (the http-push.c callsite\n> I mentioned above, and now patch 6 adds another one).\n>\n> So I think my preference would be:\n>\n> - apply the patch below as 5.5/13\n>\n> - tweak patch 9 to remove the extra trimming\n>\n> - add a patch 14 to call strbuf_trim on the name and email buffers\n> after reading them from system files.\n\nSounds sensible; thanks.\n"},{"id":"191845","messageId":"20120521230917.GA474@sigill.intra.peff.net","threadId":"30561","inReplyTo":"7v396t99up.fsf@alter.siamese.dyndns.org","subject":"[PATCHv2 0/15] ident cleanups git_default_name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:09:17Z","receivedAt":"2012-05-21T23:09:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 21, 2012 at 02:11:58AM -0700, Junio C Hamano wrote:\n\n> > So I think my preference would be:\n> >\n> > - apply the patch below as 5.5/13\n> >\n> > - tweak patch 9 to remove the extra trimming\n> >\n> > - add a patch 14 to call strbuf_trim on the name and email buffers\n> > after reading them from system files.\n> \n> Sounds sensible; thanks.\n\nHere's the whole series again. Most of it is unchanged, but given the\nordering problem earlier, it seems reasonable to just post it all again.\n\n [01/15]: ident: split setup_ident into separate functions\n [02/15]: http-push: do not access git_default_email directly\n [03/15]: fmt-merge-msg: don't use static buffer in record_person\n [04/15]: move identity config parsing to ident.c\n [05/15]: move git_default_* variables to ident.c\n [06/15]: ident: trim trailing newline from /etc/mailname\n\n This one is new and fixes the t4014 failure. The earlier version I\n posted trimmed all trailing whitespace, but there's not much point in\n being thorough, since this code gets removed later, anyway.\n\n [07/15]: format-patch: use default email for generating message ids\n [08/15]: fmt_ident: drop IDENT_WARN_ON_NO_NAME code\n [09/15]: ident: don't write fallback username into git_default_name\n [10/15]: drop length limitations on gecos-derived names and emails\n\n This one drops the new code from patch 6, as it is pointless with\n strbufs. I considered reordering the patches to avoid patch 6\n altogether, but there is a circular dependency with getting rid of the\n WARN case in patch 8.\n\n [11/15]: ident: report passwd errors with a more friendly message\n [12/15]: ident: use full dns names to generate email addresses\n [13/15]: ident: use a dynamic strbuf in fmt_ident\n [14/15]: ident: trim whitespace from default name/email\n\n This one is new, and actually does a good job of trimming unnecessary\n whitespace (from both the front and back, and from both name and\n email).\n\n [15/15]: format-patch: refactor get_patch_filename\n\n-Peff\n"},{"id":"191846","messageId":"20120521230943.GA10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 01/15] ident: split setup_ident into separate functions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:09:43Z","receivedAt":"2012-05-21T23:09:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This function sets up the default name, email, and date, and\nis not publicly available. Let's split it into three public\nfunctions so that callers can get just the parts they need.\n\nWhile we're at it, let's change the interface to simple\naccessors. The original function was called only by fmt_ident,\nand contained logic for \"if we already have some other\nvalue, don't load the default\" which properly belongs in\nfmt_ident.\n\nSigned-off-by: Jeff King \n---\n cache.h | 3 +++\n ident.c | 35 +++++++++++++++++++----------------\n 2 files changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex e14ffcd..0c095d4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -894,6 +894,9 @@ extern const char *git_author_info(int);\n extern const char *git_committer_info(int);\n extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);\n extern const char *fmt_name(const char *name, const char *email);\n+extern const char *ident_default_name(void);\n+extern const char *ident_default_email(void);\n+extern const char *ident_default_date(void);\n extern const char *git_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n \ndiff --git a/ident.c b/ident.c\nindex 87c697c..0f7dcae 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -117,21 +117,20 @@ static void copy_email(const struct passwd *pw)\n \t\t\tsizeof(git_default_email) - len);\n }\n \n-static void setup_ident(const char **name, const char **emailp)\n+const char *ident_default_name(void)\n {\n-\tstruct passwd *pw = NULL;\n-\n-\t/* Get the name (\"gecos\") */\n-\tif (!*name && !git_default_name[0]) {\n-\t\tpw = getpwuid(getuid());\n+\tif (!git_default_name[0]) {\n+\t\tstruct passwd *pw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n \t\tcopy_gecos(pw, git_default_name, sizeof(git_default_name));\n \t}\n-\tif (!*name)\n-\t\t*name = git_default_name;\n+\treturn git_default_name;\n+}\n \n-\tif (!*emailp && !git_default_email[0]) {\n+const char *ident_default_email(void)\n+{\n+\tif (!git_default_email[0]) {\n \t\tconst char *email = getenv(\"EMAIL\");\n \n \t\tif (email && email[0]) {\n@@ -139,19 +138,20 @@ static void setup_ident(const char **name, const char **emailp)\n \t\t\t\tsizeof(git_default_email));\n \t\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\t} else {\n-\t\t\tif (!pw)\n-\t\t\t\tpw = getpwuid(getuid());\n+\t\t\tstruct passwd *pw = getpwuid(getuid());\n \t\t\tif (!pw)\n \t\t\t\tdie(\"You don't exist. Go away!\");\n \t\t\tcopy_email(pw);\n \t\t}\n \t}\n-\tif (!*emailp)\n-\t\t*emailp = git_default_email;\n+\treturn git_default_email;\n+}\n \n-\t/* And set the default date */\n+const char *ident_default_date(void)\n+{\n \tif (!git_default_date[0])\n \t\tdatestamp(git_default_date, sizeof(git_default_date));\n+\treturn git_default_date;\n }\n \n static int add_raw(char *buf, size_t size, int offset, const char *str)\n@@ -311,7 +311,10 @@ const char *fmt_ident(const char *name, const char *email,\n \tint warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);\n \tint name_addr_only = (flag & IDENT_NO_DATE);\n \n-\tsetup_ident(&name, &email);\n+\tif (!name)\n+\t\tname = ident_default_name();\n+\tif (!email)\n+\t\temail = ident_default_email();\n \n \tif (!*name) {\n \t\tstruct passwd *pw;\n@@ -331,7 +334,7 @@ const char *fmt_ident(const char *name, const char *email,\n \t\tname = git_default_name;\n \t}\n \n-\tstrcpy(date, git_default_date);\n+\tstrcpy(date, ident_default_date());\n \tif (!name_addr_only && date_str && date_str[0]) {\n \t\tif (parse_date(date_str, date, sizeof(date)) < 0)\n \t\t\tdie(\"invalid date format: %s\", date_str);\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191848","messageId":"20120521230947.GB10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 02/15] http-push: do not access git_default_email directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:09:47Z","receivedAt":"2012-05-21T23:09:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"By calling the ident_default_email accessor, we can be sure\nthat the default value is actually filled-in.\n\nSigned-off-by: Jeff King \n---\n http-push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 1df7ab5..a832ca7 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -904,7 +904,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \t\tep = strchr(ep + 1, '/');\n \t}\n \n-\tescaped = xml_entities(git_default_email);\n+\tescaped = xml_entities(ident_default_email());\n \tstrbuf_addf(&out_buffer.buf, LOCK_REQUEST, escaped);\n \tfree(escaped);\n \n-- \n1.7.10.1.19.g711d603\n"},{"id":"191847","messageId":"20120521230951.GC10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 03/15] fmt-merge-msg: don't use static buffer in record_person","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:09:51Z","receivedAt":"2012-05-21T23:09:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The record_person function just parses out the \"name\" field\nof the person line in a commit and adds it to a string_list.\nThe only reason we need an extra buffer is that the\nstring_list functions require a NUL-terminated string.\n\nInstead of the static buffer, we can just allocate a\ntemporary NUL-terminated copy. In addition to removing a\nuseless limit, this removes the only user of MAX_GITNAME\noutside of ident.c.\n\nSigned-off-by: Jeff King \n---\n builtin/fmt-merge-msg.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex a517f17..4ee6a88 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -230,7 +230,7 @@ static void add_branch_desc(struct strbuf *out, const char *name)\n static void record_person(int which, struct string_list *people,\n \t\t\t struct commit *commit)\n {\n-\tchar name_buf[MAX_GITNAME], *name, *name_end;\n+\tchar *name_buf, *name, *name_end;\n \tstruct string_list_item *elem;\n \tconst char *field = (which == 'a') ? \"\\nauthor \" : \"\\ncommitter \";\n \n@@ -243,10 +243,9 @@ static void record_person(int which, struct string_list *people,\n \t\tname_end--;\n \twhile (isspace(*name_end) && name <= name_end)\n \t\tname_end--;\n-\tif (name_end < name || name + MAX_GITNAME <= name_end)\n+\tif (name_end < name)\n \t\treturn;\n-\tmemcpy(name_buf, name, name_end - name + 1);\n-\tname_buf[name_end - name + 1] = '\\0';\n+\tname_buf = xmemdupz(name, name_end - name + 1);\n \n \telem = string_list_lookup(people, name_buf);\n \tif (!elem) {\n@@ -254,6 +253,7 @@ static void record_person(int which, struct string_list *people,\n \t\telem->util = (void *)0;\n \t}\n \telem->util = (void*)(util_as_integral(elem) + 1);\n+\tfree(name_buf);\n }\n \n static int cmp_string_list_util_as_integral(const void *a_, const void *b_)\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191849","messageId":"20120521230954.GD10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 04/15] move identity config parsing to ident.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:09:54Z","receivedAt":"2012-05-21T23:09:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There's no reason for this to be in config, except that once\nupon a time all of the config parsing was there. It makes\nmore sense to keep the ident code together.\n\nSigned-off-by: Jeff King \n---\n cache.h | 1 +\n config.c | 24 +-----------------------\n ident.c | 21 +++++++++++++++++++++\n 3 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 0c095d4..86224c8 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -899,6 +899,7 @@ extern const char *ident_default_email(void);\n extern const char *ident_default_date(void);\n extern const char *git_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n+extern int git_ident_config(const char *, const char *, void *);\n \n struct ident_split {\n \tconst char *name_begin;\ndiff --git a/config.c b/config.c\nindex eeee986..71ef171 100644\n--- a/config.c\n+++ b/config.c\n@@ -762,28 +762,6 @@ static int git_default_core_config(const char *var, const char *value)\n \treturn 0;\n }\n \n-static int git_default_user_config(const char *var, const char *value)\n-{\n-\tif (!strcmp(var, \"user.name\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tstrlcpy(git_default_name, value, sizeof(git_default_name));\n-\t\tuser_ident_explicitly_given |= IDENT_NAME_GIVEN;\n-\t\treturn 0;\n-\t}\n-\n-\tif (!strcmp(var, \"user.email\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\tstrlcpy(git_default_email, value, sizeof(git_default_email));\n-\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n-\t\treturn 0;\n-\t}\n-\n-\t/* Add other config variables here and to Documentation/config.txt. */\n-\treturn 0;\n-}\n-\n static int git_default_i18n_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"i18n.commitencoding\"))\n@@ -870,7 +848,7 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \t\treturn git_default_core_config(var, value);\n \n \tif (!prefixcmp(var, \"user.\"))\n-\t\treturn git_default_user_config(var, value);\n+\t\treturn git_ident_config(var, value, dummy);\n \n \tif (!prefixcmp(var, \"i18n.\"))\n \t\treturn git_default_i18n_config(var, value);\ndiff --git a/ident.c b/ident.c\nindex 0f7dcae..bb1158f 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -388,3 +388,24 @@ int user_ident_sufficiently_given(void)\n \treturn (user_ident_explicitly_given == IDENT_ALL_GIVEN);\n #endif\n }\n+\n+int git_ident_config(const char *var, const char *value, void *data)\n+{\n+\tif (!strcmp(var, \"user.name\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tstrlcpy(git_default_name, value, sizeof(git_default_name));\n+\t\tuser_ident_explicitly_given |= IDENT_NAME_GIVEN;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(var, \"user.email\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tstrlcpy(git_default_email, value, sizeof(git_default_email));\n+\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n+\t\treturn 0;\n+\t}\n+\n+\treturn 0;\n+}\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191850","messageId":"20120521230957.GE10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 05/15] move git_default_* variables to ident.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:09:57Z","receivedAt":"2012-05-21T23:09:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There's no reason anybody outside of ident.c should access\nthese directly (they should use the new accessors which make\nsure the variables are initialized), so we can make them\nfile-scope statics.\n\nWhile we're at it, move user_ident_explicitly_given into\nident.c; while still globally visible, it makes more sense\nto reside with the ident code.\n\nSigned-off-by: Jeff King \n---\n cache.h | 3 ---\n environment.c | 3 ---\n ident.c | 4 ++++\n 3 files changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 86224c8..f63b71f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1142,9 +1142,6 @@ struct config_include_data {\n #define CONFIG_INCLUDE_INIT { 0 }\n extern int git_config_include(const char *name, const char *value, void *data);\n \n-#define MAX_GITNAME (1000)\n-extern char git_default_email[MAX_GITNAME];\n-extern char git_default_name[MAX_GITNAME];\n #define IDENT_NAME_GIVEN 01\n #define IDENT_MAIL_GIVEN 02\n #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)\ndiff --git a/environment.c b/environment.c\nindex d7e6c65..669e498 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -11,9 +11,6 @@\n #include \"refs.h\"\n #include \"fmt-merge-msg.h\"\n \n-char git_default_email[MAX_GITNAME];\n-char git_default_name[MAX_GITNAME];\n-int user_ident_explicitly_given;\n int trust_executable_bit = 1;\n int trust_ctime = 1;\n int has_symlinks = 1;\ndiff --git a/ident.c b/ident.c\nindex bb1158f..af92b2c 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -7,7 +7,11 @@\n */\n #include \"cache.h\"\n \n+#define MAX_GITNAME (1000)\n+static char git_default_name[MAX_GITNAME];\n+static char git_default_email[MAX_GITNAME];\n static char git_default_date[50];\n+int user_ident_explicitly_given;\n \n #ifdef NO_GECOS_IN_PWENT\n #define get_gecos(ignored) \"&\"\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191851","messageId":"20120521231002.GF10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 06/15] ident: trim trailing newline from /etc/mailname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:02Z","receivedAt":"2012-05-21T23:10:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We use fgets to read the /etc/mailname file, which means we\nwill typically end up with an extra newline in our\ngit_default_email. Most of the time this doesn't matter, as\nfmt_ident will skip it as cruft, but there is one code path\nthat accesses it directly (in http-push.c:lock_remote).\n\nSigned-off-by: Jeff King \n---\n ident.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/ident.c b/ident.c\nindex af92b2c..acb3a08 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -74,6 +74,10 @@ static int add_mailname_host(char *buf, size_t len)\n \t}\n \t/* success! */\n \tfclose(mailname);\n+\n+\tlen = strlen(buf);\n+\tif (len && buf[len-1] == '\\n')\n+\t\tbuf[len-1] = '\\0';\n \treturn 0;\n }\n \n-- \n1.7.10.1.19.g711d603\n"},{"id":"191852","messageId":"20120521231008.GG10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 07/15] format-patch: use default email for generating message ids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:08Z","receivedAt":"2012-05-21T23:10:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We try to generate a sane message id for cover letters and\nthreading by appending some changing bits to the front of\nthe user's email address. The current code parses the email\nout of the results of git_committer_info, but we can do this\nmuch more easily by just calling ident_default_email\nourselves.\n\nSigned-off-by: Jeff King \n---\n builtin/log.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 690caa7..656bddf 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -737,15 +737,9 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha\n \n static void gen_message_id(struct rev_info *info, char *base)\n {\n-\tconst char *committer = git_committer_info(IDENT_WARN_ON_NO_NAME);\n-\tconst char *email_start = strrchr(committer, '<');\n-\tconst char *email_end = strrchr(committer, '>');\n \tstruct strbuf buf = STRBUF_INIT;\n-\tif (!email_start || !email_end || email_start > email_end - 1)\n-\t\tdie(_(\"Could not extract email from committer identity.\"));\n-\tstrbuf_addf(&buf, \"%s.%lu.git.%.*s\", base,\n-\t\t (unsigned long) time(NULL),\n-\t\t (int)(email_end - email_start - 1), email_start + 1);\n+\tstrbuf_addf(&buf, \"%s.%lu.git.%s\", base,\n+\t\t (unsigned long) time(NULL), ident_default_email());\n \tinfo->message_id = strbuf_detach(&buf, NULL);\n }\n \n-- \n1.7.10.1.19.g711d603\n"},{"id":"191853","messageId":"20120521231011.GH10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 08/15] fmt_ident: drop IDENT_WARN_ON_NO_NAME code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:11Z","receivedAt":"2012-05-21T23:10:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are no more callers who want this, so we can drop it.\n\nSigned-off-by: Jeff King \n---\n cache.h | 5 ++---\n ident.c | 11 ++++-------\n 2 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex f63b71f..65cbab5 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -887,9 +887,8 @@ unsigned long approxidate_careful(const char *, int *);\n unsigned long approxidate_relative(const char *date, const struct timeval *now);\n enum date_mode parse_date_format(const char *format);\n \n-#define IDENT_WARN_ON_NO_NAME 1\n-#define IDENT_ERROR_ON_NO_NAME 2\n-#define IDENT_NO_DATE\t 4\n+#define IDENT_ERROR_ON_NO_NAME 1\n+#define IDENT_NO_DATE\t 2\n extern const char *git_author_info(int);\n extern const char *git_committer_info(int);\n extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);\ndiff --git a/ident.c b/ident.c\nindex acb3a08..3b92c44 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -316,7 +316,6 @@ const char *fmt_ident(const char *name, const char *email,\n \tchar date[50];\n \tint i;\n \tint error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);\n-\tint warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);\n \tint name_addr_only = (flag & IDENT_NO_DATE);\n \n \tif (!name)\n@@ -327,13 +326,11 @@ const char *fmt_ident(const char *name, const char *email,\n \tif (!*name) {\n \t\tstruct passwd *pw;\n \n-\t\tif ((warn_on_no_name || error_on_no_name) &&\n-\t\t name == git_default_name && env_hint) {\n-\t\t\tfputs(env_hint, stderr);\n-\t\t\tenv_hint = NULL; /* warn only once */\n-\t\t}\n-\t\tif (error_on_no_name)\n+\t\tif (error_on_no_name) {\n+\t\t\tif (name == git_default_name)\n+\t\t\t\tfputs(env_hint, stderr);\n \t\t\tdie(\"empty ident %s <%s> not allowed\", name, email);\n+\t\t}\n \t\tpw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191854","messageId":"20120521231014.GI10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 09/15] ident: don't write fallback username into git_default_name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:14Z","receivedAt":"2012-05-21T23:10:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The fmt_ident function gets a flag that tells us whether to\ndie if the name field is blank. If it is blank and we don't\ndie, then we fall back to the username from the passwd file.\n\nThe current code writes the value into git_default_name.\nHowever, that's not necessarily correct, as the empty value\nmight have come from git_default_name, or it might have been\npassed in. This leads to two potential problems:\n\n 1. If we are overriding an empty name in the passed-in\n value, then we may be overwriting a perfectly good name\n (from gitconfig or gecos) in the git_default_name\n buffer. Later calls to fmt_ident will end up using the\n fallback name, even though a better name was available.\n\n 2. If we override an empty gecos name, we end up with the\n fallback name in git_default_name. A later call that\n uses IDENT_ERROR_ON_NO_NAME will see the fallback name\n and think that it is a good name, instead of producing\n an error. In other words, a blank gecos name would\n cause an error with this code:\n\n git_committer_info(IDENT_ERROR_ON_NO_NAME);\n\n but not this:\n\n git_committer_info(0);\n git_committer_info(IDENT_ERROR_ON_NO_NAME);\n\n because in the latter case, the first call has polluted\n the name buffer.\n\nInstead, let's make the fallback a per-invocation variable.\nWe can just use the pw->pw_name string directly, since it\nonly needs to persist through the rest of the function (and\nwe don't do any other getpwent calls).\n\nNote that while this solves (1) for future invocations of\nfmt_indent, the current invocation might use the fallback\nwhen it could in theory load a better value from\ngit_default_name. However, by not passing\nIDENT_ERROR_ON_NO_NAME, the caller is indicating that it\ndoes not care too much about the name, anyway, so we don't\nbother; this is primarily about protecting future callers\nwho do care.\n\nSigned-off-by: Jeff King \n---\n ident.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 3b92c44..87e3cbe 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -334,9 +334,7 @@ const char *fmt_ident(const char *name, const char *email,\n \t\tpw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n-\t\tstrlcpy(git_default_name, pw->pw_name,\n-\t\t\tsizeof(git_default_name));\n-\t\tname = git_default_name;\n+\t\tname = pw->pw_name;\n \t}\n \n \tstrcpy(date, ident_default_date());\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191860","messageId":"20120521231017.GJ10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 10/15] drop length limitations on gecos-derived names and emails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:17Z","receivedAt":"2012-05-21T23:10:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we pull the user's name from the GECOS field of the\npasswd file (or generate an email address based on their\nusername and hostname), we put the result into a\nstatic buffer. While it's extremely unlikely that anybody\never hit these limits (after all, in such a case their\nparents must have hated them), we still had to deal with the\nerror cases in our code.\n\nConverting these static buffers to strbufs lets us simplify\nthe code and drop some error messages from the documentation\nthat have confused some users.\n\nThe conversion is mostly mechanical: replace string copies\nwith strbuf equivalents, and access the strbuf.buf directly.\nThere are a few exceptions:\n\n - copy_gecos and copy_email are the big winners in code\n reduction (since they no longer have to manage the\n string length manually)\n\n - git_ident_config wants to replace old versions of\n the default name (e.g., if we read the config multiple\n times), so it must reset+add to the strbuf instead of\n just adding\n\nNote that there is still one length limitation: the\ngethostname interface requires us to provide a static\nbuffer, so we arbitrarily choose 1024 bytes for the\nhostname.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-commit-tree.txt | 4 --\n Documentation/git-var.txt | 4 --\n ident.c | 104 ++++++++++++++------------------------\n 3 files changed, 39 insertions(+), 73 deletions(-)\n\ndiff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt\nindex cfb9906..eb12b2d 100644\n--- a/Documentation/git-commit-tree.txt\n+++ b/Documentation/git-commit-tree.txt\n@@ -92,10 +92,6 @@ Diagnostics\n -----------\n You don't exist. Go away!::\n The passwd(5) gecos field couldn't be read\n-Your parents must have hated you!::\n- The passwd(5) gecos field is longer than a giant static buffer.\n-Your sysadmin must hate you!::\n- The passwd(5) name field is longer than a giant static buffer.\n \n Discussion\n ----------\ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex 988a323..3f703e3 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -63,10 +63,6 @@ Diagnostics\n -----------\n You don't exist. Go away!::\n The passwd(5) gecos field couldn't be read\n-Your parents must have hated you!::\n- The passwd(5) gecos field is longer than a giant static buffer.\n-Your sysadmin must hate you!::\n- The passwd(5) name field is longer than a giant static buffer.\n \n SEE ALSO\n --------\ndiff --git a/ident.c b/ident.c\nindex 87e3cbe..73a06a1 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -7,9 +7,8 @@\n */\n #include \"cache.h\"\n \n-#define MAX_GITNAME (1000)\n-static char git_default_name[MAX_GITNAME];\n-static char git_default_email[MAX_GITNAME];\n+static struct strbuf git_default_name = STRBUF_INIT;\n+static struct strbuf git_default_email = STRBUF_INIT;\n static char git_default_date[50];\n int user_ident_explicitly_given;\n \n@@ -19,42 +18,27 @@ int user_ident_explicitly_given;\n #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)\n #endif\n \n-static void copy_gecos(const struct passwd *w, char *name, size_t sz)\n+static void copy_gecos(const struct passwd *w, struct strbuf *name)\n {\n-\tchar *src, *dst;\n-\tsize_t len, nlen;\n-\n-\tnlen = strlen(w->pw_name);\n+\tchar *src;\n \n \t/* Traditionally GECOS field had office phone numbers etc, separated\n \t * with commas. Also & stands for capitalized form of the login name.\n \t */\n \n-\tfor (len = 0, dst = name, src = get_gecos(w); len < sz; src++) {\n+\tfor (src = get_gecos(w); *src && *src != ','; src++) {\n \t\tint ch = *src;\n-\t\tif (ch != '&') {\n-\t\t\t*dst++ = ch;\n-\t\t\tif (ch == 0 || ch == ',')\n-\t\t\t\tbreak;\n-\t\t\tlen++;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (len + nlen < sz) {\n+\t\tif (ch != '&')\n+\t\t\tstrbuf_addch(name, ch);\n+\t\telse {\n \t\t\t/* Sorry, Mr. McDonald... */\n-\t\t\t*dst++ = toupper(*w->pw_name);\n-\t\t\tmemcpy(dst, w->pw_name + 1, nlen - 1);\n-\t\t\tdst += nlen - 1;\n-\t\t\tlen += nlen;\n+\t\t\tstrbuf_addch(name, toupper(*w->pw_name));\n+\t\t\tstrbuf_addstr(name, w->pw_name + 1);\n \t\t}\n \t}\n-\tif (len < sz)\n-\t\tname[len] = 0;\n-\telse\n-\t\tdie(\"Your parents must have hated you!\");\n-\n }\n \n-static int add_mailname_host(char *buf, size_t len)\n+static int add_mailname_host(struct strbuf *buf)\n {\n \tFILE *mailname;\n \n@@ -65,7 +49,7 @@ static int add_mailname_host(char *buf, size_t len)\n \t\t\t\tstrerror(errno));\n \t\treturn -1;\n \t}\n-\tif (!fgets(buf, len, mailname)) {\n+\tif (strbuf_getline(buf, mailname, '\\n') == EOF) {\n \t\tif (ferror(mailname))\n \t\t\twarning(\"cannot read /etc/mailname: %s\",\n \t\t\t\tstrerror(errno));\n@@ -74,85 +58,73 @@ static int add_mailname_host(char *buf, size_t len)\n \t}\n \t/* success! */\n \tfclose(mailname);\n-\n-\tlen = strlen(buf);\n-\tif (len && buf[len-1] == '\\n')\n-\t\tbuf[len-1] = '\\0';\n \treturn 0;\n }\n \n-static void add_domainname(char *buf, size_t len)\n+static void add_domainname(struct strbuf *out)\n {\n+\tchar buf[1024];\n \tstruct hostent *he;\n-\tsize_t namelen;\n \tconst char *domainname;\n \n-\tif (gethostname(buf, len)) {\n+\tif (gethostname(buf, sizeof(buf))) {\n \t\twarning(\"cannot get host name: %s\", strerror(errno));\n-\t\tstrlcpy(buf, \"(none)\", len);\n+\t\tstrbuf_addstr(out, \"(none)\");\n \t\treturn;\n \t}\n-\tnamelen = strlen(buf);\n-\tif (memchr(buf, '.', namelen))\n+\tstrbuf_addstr(out, buf);\n+\tif (strchr(buf, '.'))\n \t\treturn;\n \n \the = gethostbyname(buf);\n-\tbuf[namelen++] = '.';\n-\tbuf += namelen;\n-\tlen -= namelen;\n+\tstrbuf_addch(out, '.');\n \tif (he && (domainname = strchr(he->h_name, '.')))\n-\t\tstrlcpy(buf, domainname + 1, len);\n+\t\tstrbuf_addstr(out, domainname + 1);\n \telse\n-\t\tstrlcpy(buf, \"(none)\", len);\n+\t\tstrbuf_addstr(out, \"(none)\");\n }\n \n-static void copy_email(const struct passwd *pw)\n+static void copy_email(const struct passwd *pw, struct strbuf *email)\n {\n \t/*\n \t * Make up a fake email address\n \t * (name + '@' + hostname [+ '.' + domainname])\n \t */\n-\tsize_t len = strlen(pw->pw_name);\n-\tif (len > sizeof(git_default_email)/2)\n-\t\tdie(\"Your sysadmin must hate you!\");\n-\tmemcpy(git_default_email, pw->pw_name, len);\n-\tgit_default_email[len++] = '@';\n-\n-\tif (!add_mailname_host(git_default_email + len,\n-\t\t\t\tsizeof(git_default_email) - len))\n+\tstrbuf_addstr(email, pw->pw_name);\n+\tstrbuf_addch(email, '@');\n+\n+\tif (!add_mailname_host(email))\n \t\treturn;\t/* read from \"/etc/mailname\" (Debian) */\n-\tadd_domainname(git_default_email + len,\n-\t\t\tsizeof(git_default_email) - len);\n+\tadd_domainname(email);\n }\n \n const char *ident_default_name(void)\n {\n-\tif (!git_default_name[0]) {\n+\tif (!git_default_name.len) {\n \t\tstruct passwd *pw = getpwuid(getuid());\n \t\tif (!pw)\n \t\t\tdie(\"You don't exist. Go away!\");\n-\t\tcopy_gecos(pw, git_default_name, sizeof(git_default_name));\n+\t\tcopy_gecos(pw, &git_default_name);\n \t}\n-\treturn git_default_name;\n+\treturn git_default_name.buf;\n }\n \n const char *ident_default_email(void)\n {\n-\tif (!git_default_email[0]) {\n+\tif (!git_default_email.len) {\n \t\tconst char *email = getenv(\"EMAIL\");\n \n \t\tif (email && email[0]) {\n-\t\t\tstrlcpy(git_default_email, email,\n-\t\t\t\tsizeof(git_default_email));\n+\t\t\tstrbuf_addstr(&git_default_email, email);\n \t\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\t} else {\n \t\t\tstruct passwd *pw = getpwuid(getuid());\n \t\t\tif (!pw)\n \t\t\t\tdie(\"You don't exist. Go away!\");\n-\t\t\tcopy_email(pw);\n+\t\t\tcopy_email(pw, &git_default_email);\n \t\t}\n \t}\n-\treturn git_default_email;\n+\treturn git_default_email.buf;\n }\n \n const char *ident_default_date(void)\n@@ -327,7 +299,7 @@ const char *fmt_ident(const char *name, const char *email,\n \t\tstruct passwd *pw;\n \n \t\tif (error_on_no_name) {\n-\t\t\tif (name == git_default_name)\n+\t\t\tif (name == git_default_name.buf)\n \t\t\t\tfputs(env_hint, stderr);\n \t\t\tdie(\"empty ident %s <%s> not allowed\", name, email);\n \t\t}\n@@ -397,7 +369,8 @@ int git_ident_config(const char *var, const char *value, void *data)\n \tif (!strcmp(var, \"user.name\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tstrlcpy(git_default_name, value, sizeof(git_default_name));\n+\t\tstrbuf_reset(&git_default_name);\n+\t\tstrbuf_addstr(&git_default_name, value);\n \t\tuser_ident_explicitly_given |= IDENT_NAME_GIVEN;\n \t\treturn 0;\n \t}\n@@ -405,7 +378,8 @@ int git_ident_config(const char *var, const char *value, void *data)\n \tif (!strcmp(var, \"user.email\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tstrlcpy(git_default_email, value, sizeof(git_default_email));\n+\t\tstrbuf_reset(&git_default_email);\n+\t\tstrbuf_addstr(&git_default_email, value);\n \t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\treturn 0;\n \t}\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191859","messageId":"20120521231020.GK10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 11/15] ident: report passwd errors with a more friendly message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:20Z","receivedAt":"2012-05-21T23:10:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When getpwuid fails, we give a cute but cryptic message.\nWhile it makes sense if you know that getpwuid or identity\nfunctions are being called, this code is triggered behind\nthe scenes by quite a few git commands these days (e.g.,\nreceive-pack on a remote server might use it for a reflog;\nthe current message is hard to distinguish from an\nauthentication error). Let's switch to something that gives\na little more context.\n\nWhile we're at it, we can factor out all of the\ncut-and-pastes of the \"you don't exist\" message into a\nwrapper function. Rather than provide xgetpwuid, let's make\nit even more specific to just getting the passwd entry for\nthe current uid. That's the only way we use getpwuid anyway,\nand it lets us make an even more specific error message.\n\nThe current message also fails to mention errno. While the\nusual cause for getpwuid failing is that the user does not\nexist, mentioning errno makes it easier to diagnose these\nproblems. Note that POSIX specifies that errno remain\nuntouched if the passwd entry does not exist (but will be\nset on actual errors), whereas some systems will return\nENOENT or similar for a missing entry. We handle both cases\nin our wrapper.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-commit-tree.txt | 5 -----\n Documentation/git-var.txt | 5 -----\n git-compat-util.h | 3 +++\n ident.c | 20 +++++---------------\n wrapper.c | 12 ++++++++++++\n 5 files changed, 20 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt\nindex eb12b2d..eb8ee99 100644\n--- a/Documentation/git-commit-tree.txt\n+++ b/Documentation/git-commit-tree.txt\n@@ -88,11 +88,6 @@ for one to be entered and terminated with ^D.\n \n include::date-formats.txt[]\n \n-Diagnostics\n------------\n-You don't exist. Go away!::\n- The passwd(5) gecos field couldn't be read\n-\n Discussion\n ----------\n \ndiff --git a/Documentation/git-var.txt b/Documentation/git-var.txt\nindex 3f703e3..67edf58 100644\n--- a/Documentation/git-var.txt\n+++ b/Documentation/git-var.txt\n@@ -59,11 +59,6 @@ ifdef::git-default-pager[]\n The build you are using chose '{git-default-pager}' as the default.\n endif::git-default-pager[]\n \n-Diagnostics\n------------\n-You don't exist. Go away!::\n- The passwd(5) gecos field couldn't be read\n-\n SEE ALSO\n --------\n linkgit:git-commit-tree[1]\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ed11ad8..5bd9ad7 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -595,4 +595,7 @@ int rmdir_or_warn(const char *path);\n */\n int remove_or_warn(unsigned int mode, const char *path);\n \n+/* Get the passwd entry for the UID of the current process. */\n+struct passwd *xgetpwuid_self(void);\n+\n #endif\ndiff --git a/ident.c b/ident.c\nindex 73a06a1..5aec073 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -100,12 +100,8 @@ static void copy_email(const struct passwd *pw, struct strbuf *email)\n \n const char *ident_default_name(void)\n {\n-\tif (!git_default_name.len) {\n-\t\tstruct passwd *pw = getpwuid(getuid());\n-\t\tif (!pw)\n-\t\t\tdie(\"You don't exist. Go away!\");\n-\t\tcopy_gecos(pw, &git_default_name);\n-\t}\n+\tif (!git_default_name.len)\n+\t\tcopy_gecos(xgetpwuid_self(), &git_default_name);\n \treturn git_default_name.buf;\n }\n \n@@ -117,12 +113,8 @@ const char *ident_default_email(void)\n \t\tif (email && email[0]) {\n \t\t\tstrbuf_addstr(&git_default_email, email);\n \t\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n-\t\t} else {\n-\t\t\tstruct passwd *pw = getpwuid(getuid());\n-\t\t\tif (!pw)\n-\t\t\t\tdie(\"You don't exist. Go away!\");\n-\t\t\tcopy_email(pw, &git_default_email);\n-\t\t}\n+\t\t} else\n+\t\t\tcopy_email(xgetpwuid_self(), &git_default_email);\n \t}\n \treturn git_default_email.buf;\n }\n@@ -303,9 +295,7 @@ const char *fmt_ident(const char *name, const char *email,\n \t\t\t\tfputs(env_hint, stderr);\n \t\t\tdie(\"empty ident %s <%s> not allowed\", name, email);\n \t\t}\n-\t\tpw = getpwuid(getuid());\n-\t\tif (!pw)\n-\t\t\tdie(\"You don't exist. Go away!\");\n+\t\tpw = xgetpwuid_self();\n \t\tname = pw->pw_name;\n \t}\n \ndiff --git a/wrapper.c b/wrapper.c\nindex 6ccd059..b5e33e4 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -402,3 +402,15 @@ int remove_or_warn(unsigned int mode, const char *file)\n {\n \treturn S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file);\n }\n+\n+struct passwd *xgetpwuid_self(void)\n+{\n+\tstruct passwd *pw;\n+\n+\terrno = 0;\n+\tpw = getpwuid(getuid());\n+\tif (!pw)\n+\t\tdie(_(\"unable to look up current user in the passwd file: %s\"),\n+\t\t errno ? strerror(errno) : _(\"no such user\"));\n+\treturn pw;\n+}\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191858","messageId":"20120521231023.GL10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 12/15] ident: use full dns names to generate email addresses","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:23Z","receivedAt":"2012-05-21T23:10:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we construct an email address from the username and\nhostname, we generate the host part of the email with this\nprocedure:\n\n 1. add the result of gethostname\n\n 2. if it has a dot, ok, it's fully qualified\n\n 3. if not, then look up the unqualified hostname via\n gethostbyname; take the domain name of the result and\n append it to the hostname\n\nStep 3 can actually produce a bogus result, as the name\nreturned by gethostbyname may not be related to the hostname\nwe fed it (e.g., consider a machine \"foo\" with names\n\"foo.one.example.com\" and \"bar.two.example.com\"; we may have\nthe latter returned and generate the bogus name\n\"foo.two.example.com\").\n\nThis patch simply uses the full hostname returned by\ngethostbyname. In the common case that the first part is the\nsame as the unqualified hostname, the behavior is identical.\nAnd in the case that it is not the same, we are much more\nlikely to be generating a valid name.\n\nSigned-off-by: Jeff King \n---\n ident.c | 13 ++++---------\n 1 file changed, 4 insertions(+), 9 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 5aec073..b111e34 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -65,23 +65,18 @@ static void add_domainname(struct strbuf *out)\n {\n \tchar buf[1024];\n \tstruct hostent *he;\n-\tconst char *domainname;\n \n \tif (gethostname(buf, sizeof(buf))) {\n \t\twarning(\"cannot get host name: %s\", strerror(errno));\n \t\tstrbuf_addstr(out, \"(none)\");\n \t\treturn;\n \t}\n-\tstrbuf_addstr(out, buf);\n \tif (strchr(buf, '.'))\n-\t\treturn;\n-\n-\the = gethostbyname(buf);\n-\tstrbuf_addch(out, '.');\n-\tif (he && (domainname = strchr(he->h_name, '.')))\n-\t\tstrbuf_addstr(out, domainname + 1);\n+\t\tstrbuf_addstr(out, buf);\n+\telse if ((he = gethostbyname(buf)) && strchr(he->h_name, '.'))\n+\t\tstrbuf_addstr(out, he->h_name);\n \telse\n-\t\tstrbuf_addstr(out, \"(none)\");\n+\t\tstrbuf_addf(out, \"%s.(none)\", buf);\n }\n \n static void copy_email(const struct passwd *pw, struct strbuf *email)\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191855","messageId":"20120521231026.GM10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 13/15] ident: use a dynamic strbuf in fmt_ident","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:26Z","receivedAt":"2012-05-21T23:10:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that we accept arbitrary-sized names and email\naddresses, the only remaining limit is in the actual\nformatting of the names into a buffer. The current limit is\n1000 characters, which is not likely to be reached, but\nusing a strbuf is one less error condition we have to worry\nabout.\n\nSigned-off-by: Jeff King \n---\n ident.c | 43 +++++++++++++++----------------------------\n 1 file changed, 15 insertions(+), 28 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex b111e34..cefb829 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -121,15 +121,6 @@ const char *ident_default_date(void)\n \treturn git_default_date;\n }\n \n-static int add_raw(char *buf, size_t size, int offset, const char *str)\n-{\n-\tsize_t len = strlen(str);\n-\tif (offset + len > size)\n-\t\treturn size;\n-\tmemcpy(buf + offset, str, len);\n-\treturn offset + len;\n-}\n-\n static int crud(unsigned char c)\n {\n \treturn c <= 32 ||\n@@ -148,7 +139,7 @@ static int crud(unsigned char c)\n * Copy over a string to the destination, but avoid special\n * characters ('\\n', '<' and '>') and remove crud at the end\n */\n-static int copy(char *buf, size_t size, int offset, const char *src)\n+static void strbuf_addstr_without_crud(struct strbuf *sb, const char *src)\n {\n \tsize_t i, len;\n \tunsigned char c;\n@@ -172,19 +163,19 @@ static int copy(char *buf, size_t size, int offset, const char *src)\n \t/*\n \t * Copy the rest to the buffer, but avoid the special\n \t * characters '\\n' '<' and '>' that act as delimiters on\n-\t * an identification line\n+\t * an identification line. We can only remove crud, never add it,\n+\t * so 'len' is our maximum.\n \t */\n+\tstrbuf_grow(sb, len);\n \tfor (i = 0; i < len; i++) {\n \t\tc = *src++;\n \t\tswitch (c) {\n \t\tcase '\\n': case '<': case '>':\n \t\t\tcontinue;\n \t\t}\n-\t\tif (offset >= size)\n-\t\t\treturn size;\n-\t\tbuf[offset++] = c;\n+\t\tsb->buf[sb->len++] = c;\n \t}\n-\treturn offset;\n+\tsb->buf[sb->len] = '\\0';\n }\n \n /*\n@@ -271,9 +262,8 @@ static const char *env_hint =\n const char *fmt_ident(const char *name, const char *email,\n \t\t const char *date_str, int flag)\n {\n-\tstatic char buffer[1000];\n+\tstatic struct strbuf ident = STRBUF_INIT;\n \tchar date[50];\n-\tint i;\n \tint error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);\n \tint name_addr_only = (flag & IDENT_NO_DATE);\n \n@@ -300,19 +290,16 @@ const char *fmt_ident(const char *name, const char *email,\n \t\t\tdie(\"invalid date format: %s\", date_str);\n \t}\n \n-\ti = copy(buffer, sizeof(buffer), 0, name);\n-\ti = add_raw(buffer, sizeof(buffer), i, \" <\");\n-\ti = copy(buffer, sizeof(buffer), i, email);\n+\tstrbuf_reset(&ident);\n+\tstrbuf_addstr_without_crud(&ident, name);\n+\tstrbuf_addstr(&ident, \" <\");\n+\tstrbuf_addstr_without_crud(&ident, email);\n+\tstrbuf_addch(&ident, '>');\n \tif (!name_addr_only) {\n-\t\ti = add_raw(buffer, sizeof(buffer), i, \"> \");\n-\t\ti = copy(buffer, sizeof(buffer), i, date);\n-\t} else {\n-\t\ti = add_raw(buffer, sizeof(buffer), i, \">\");\n+\t\tstrbuf_addch(&ident, ' ');\n+\t\tstrbuf_addstr_without_crud(&ident, date);\n \t}\n-\tif (i >= sizeof(buffer))\n-\t\tdie(\"Impossibly long personal identifier\");\n-\tbuffer[i] = 0;\n-\treturn buffer;\n+\treturn ident.buf;\n }\n \n const char *fmt_name(const char *name, const char *email)\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191856","messageId":"20120521231029.GN10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 14/15] ident: trim whitespace from default name/email","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:29Z","receivedAt":"2012-05-21T23:10:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Usually these values get fed to fmt_ident, which will trim\nany cruft anyway, but there are a few code paths which use\nthem directly. Let's clean them up for the benefit of those\ncallers. Furthermore, fmt_ident will look at the pre-trimmed\nvalue and decide whether to invoke ERROR_ON_NO_NAME; this\ncheck can be fooled by a name consisting only of spaces.\n\nNote that we only bother to clean up when we are pulling the\ninformation from gecos or from system files. Any other value\ncomes from a config file, where we will have cleaned up\naccidental whitespace already.\n\nSigned-off-by: Jeff King \n---\n ident.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/ident.c b/ident.c\nindex cefb829..e279039 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -95,8 +95,10 @@ static void copy_email(const struct passwd *pw, struct strbuf *email)\n \n const char *ident_default_name(void)\n {\n-\tif (!git_default_name.len)\n+\tif (!git_default_name.len) {\n \t\tcopy_gecos(xgetpwuid_self(), &git_default_name);\n+\t\tstrbuf_trim(&git_default_name);\n+\t}\n \treturn git_default_name.buf;\n }\n \n@@ -110,6 +112,7 @@ const char *ident_default_email(void)\n \t\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\t} else\n \t\t\tcopy_email(xgetpwuid_self(), &git_default_email);\n+\t\tstrbuf_trim(&git_default_email);\n \t}\n \treturn git_default_email.buf;\n }\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191857","messageId":"20120521231032.GO10981@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20120521230917.GA474@sigill.intra.peff.net","subject":"[PATCHv2 15/15] format-patch: refactor get_patch_filename","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-21T23:10:32Z","receivedAt":"2012-05-21T23:10:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The get_patch_filename function expects a commit argument\nand uses it to get the sanitized subject line when making a\npatch filename. However, we also want to use this same\nfunction for the cover letter, which does not have a commit\nobject. The current solution is to create a fake commit with\nthe subject \"cover letter\". Instead, let's make the\nget_patch_filename interface more flexibile, and allow\npassing a direct subject.\n\nSigned-off-by: Jeff King \n---\n builtin/log.c | 35 +++++++----------------------------\n log-tree.c | 19 +++++++++++--------\n log-tree.h | 4 ++--\n 3 files changed, 20 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 656bddf..8010a40 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -663,7 +663,8 @@ static FILE *realstdout = NULL;\n static const char *output_directory = NULL;\n static int outdir_offset;\n \n-static int reopen_stdout(struct commit *commit, struct rev_info *rev, int quiet)\n+static int reopen_stdout(struct commit *commit, const char *subject,\n+\t\t\t struct rev_info *rev, int quiet)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n \tint suffix_len = strlen(fmt_patch_suffix) + 1;\n@@ -677,7 +678,7 @@ static int reopen_stdout(struct commit *commit, struct rev_info *rev, int quiet)\n \t\t\tstrbuf_addch(&filename, '/');\n \t}\n \n-\tget_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);\n+\tget_patch_filename(commit, subject, rev->nr, fmt_patch_suffix, &filename);\n \n \tif (!quiet)\n \t\tfprintf(realstdout, \"%s\\n\", filename.buf + outdir_offset);\n@@ -778,7 +779,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tconst char *encoding = \"UTF-8\";\n \tstruct diff_options opts;\n \tint need_8bit_cte = 0;\n-\tstruct commit *commit = NULL;\n \tstruct pretty_print_context pp = {0};\n \n \tif (rev->commit_format != CMIT_FMT_EMAIL)\n@@ -786,31 +786,10 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \n \tcommitter = git_committer_info(0);\n \n-\tif (!numbered_files) {\n-\t\t/*\n-\t\t * We fake a commit for the cover letter so we get the filename\n-\t\t * desired.\n-\t\t */\n-\t\tcommit = xcalloc(1, sizeof(*commit));\n-\t\tcommit->buffer = xmalloc(400);\n-\t\tsnprintf(commit->buffer, 400,\n-\t\t\t\"tree 0000000000000000000000000000000000000000\\n\"\n-\t\t\t\"parent %s\\n\"\n-\t\t\t\"author %s\\n\"\n-\t\t\t\"committer %s\\n\\n\"\n-\t\t\t\"cover letter\\n\",\n-\t\t\tsha1_to_hex(head->object.sha1), committer, committer);\n-\t}\n-\n-\tif (!use_stdout && reopen_stdout(commit, rev, quiet))\n+\tif (!use_stdout &&\n+\t reopen_stdout(NULL, numbered_files ? NULL : \"cover-letter\", rev, quiet))\n \t\treturn;\n \n-\tif (commit) {\n-\n-\t\tfree(commit->buffer);\n-\t\tfree(commit);\n-\t}\n-\n \tlog_write_email_headers(rev, head, &pp.subject, &pp.after_subject,\n \t\t\t\t&need_8bit_cte);\n \n@@ -1405,8 +1384,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tgen_message_id(&rev, sha1_to_hex(commit->object.sha1));\n \t\t}\n \n-\t\tif (!use_stdout && reopen_stdout(numbered_files ? NULL : commit,\n-\t\t\t\t\t\t &rev, quiet))\n+\t\tif (!use_stdout &&\n+\t\t reopen_stdout(numbered_files ? NULL : commit, NULL, &rev, quiet))\n \t\t\tdie(_(\"Failed to create output files\"));\n \t\tshown = log_tree_commit(&rev, commit);\n \t\tfree(commit->buffer);\ndiff --git a/log-tree.c b/log-tree.c\nindex 376d973..c894930 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -299,19 +299,22 @@ static unsigned int digits_in_number(unsigned int number)\n \treturn result;\n }\n \n-void get_patch_filename(struct commit *commit, int nr, const char *suffix,\n-\t\t\tstruct strbuf *buf)\n+void get_patch_filename(struct commit *commit, const char *subject, int nr,\n+\t\t\tconst char *suffix, struct strbuf *buf)\n {\n \tint suffix_len = strlen(suffix) + 1;\n \tint start_len = buf->len;\n \n-\tstrbuf_addf(buf, commit ? \"%04d-\" : \"%d\", nr);\n-\tif (commit) {\n+\tstrbuf_addf(buf, commit || subject ? \"%04d-\" : \"%d\", nr);\n+\tif (commit || subject) {\n \t\tint max_len = start_len + FORMAT_PATCH_NAME_MAX - suffix_len;\n \t\tstruct pretty_print_context ctx = {0};\n-\t\tctx.date_mode = DATE_NORMAL;\n \n-\t\tformat_commit_message(commit, \"%f\", buf, &ctx);\n+\t\tif (subject)\n+\t\t\tstrbuf_addstr(buf, subject);\n+\t\telse if (commit)\n+\t\t\tformat_commit_message(commit, \"%f\", buf, &ctx);\n+\n \t\tif (max_len < buf->len)\n \t\t\tstrbuf_setlen(buf, max_len);\n \t\tstrbuf_addstr(buf, suffix);\n@@ -384,8 +387,8 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n \t\t\t mime_boundary_leader, opt->mime_boundary);\n \t\textra_headers = subject_buffer;\n \n-\t\tget_patch_filename(opt->numbered_files ? NULL : commit, opt->nr,\n-\t\t\t\t opt->patch_suffix, &filename);\n+\t\tget_patch_filename(opt->numbered_files ? NULL : commit, NULL,\n+\t\t\t\t opt->nr, opt->patch_suffix, &filename);\n \t\tsnprintf(buffer, sizeof(buffer) - 1,\n \t\t\t \"\\n--%s%s\\n\"\n \t\t\t \"Content-Type: text/x-patch;\"\ndiff --git a/log-tree.h b/log-tree.h\nindex 5c4cf7c..f5ac238 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -21,7 +21,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n void load_ref_decorations(int flags);\n \n #define FORMAT_PATCH_NAME_MAX 64\n-void get_patch_filename(struct commit *commit, int nr, const char *suffix,\n-\t\t\tstruct strbuf *buf);\n+void get_patch_filename(struct commit *commit, const char *subject, int nr,\n+\t\t\tconst char *suffix, struct strbuf *buf);\n \n #endif\n-- \n1.7.10.1.19.g711d603\n"},{"id":"191902","messageId":"7vipfo5f60.fsf@alter.siamese.dyndns.org","threadId":"30561","inReplyTo":"20120521231029.GN10981@sigill.intra.peff.net","subject":"Re: [PATCHv2 14/15] ident: trim whitespace from default name/email","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-22T16:55:19Z","receivedAt":"2012-05-22T16:55:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ... Any other value\n> comes from a config file, where we will have cleaned up\n> accidental whitespace already.\n\nAre you referring to the behaviour of the config parser that removes\nleading and trailing whitespaces when reading an unquoted string value?\n\nIf the user really wanted to have trailing whitespaces by quoting, we\nwould let it pass, in other words; it probably is a reasonable behaviour.\n\nThe same can be said about the environment variable GIT_COMMITTER_NAME and\nfriends, but \"accidental whitespaces are cleaned up already\" does not\napply to them.\n\nSo, isn't the real rationale behind this choice to allow users who give\nleading or trailing whitespaces in the configuration and environment\nvariables on purpose use whatever value they specified?\n\nI agree with the placement of trimming in this patch, but I do not quite\nget (I do not mean \"I do not agree with\") what the quoted sentence wanted\nto say.\n\nOther than that single small \"hrm...\", the entire series was a pleasant\nread. Thanks.\n\n>\n> Signed-off-by: Jeff King \n> ---\n> ident.c | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/ident.c b/ident.c\n> index cefb829..e279039 100644\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -95,8 +95,10 @@ static void copy_email(const struct passwd *pw, struct strbuf *email)\n> \n> const char *ident_default_name(void)\n> {\n> -\tif (!git_default_name.len)\n> +\tif (!git_default_name.len) {\n> \t\tcopy_gecos(xgetpwuid_self(), &git_default_name);\n> +\t\tstrbuf_trim(&git_default_name);\n> +\t}\n> \treturn git_default_name.buf;\n> }\n> \n> @@ -110,6 +112,7 @@ const char *ident_default_email(void)\n> \t\t\tuser_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n> \t\t} else\n> \t\t\tcopy_email(xgetpwuid_self(), &git_default_email);\n> +\t\tstrbuf_trim(&git_default_email);\n> \t}\n> \treturn git_default_email.buf;\n> }\n"},{"id":"191904","messageId":"20120522171204.GB11600@sigill.intra.peff.net","threadId":"30561","inReplyTo":"7vipfo5f60.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 14/15] ident: trim whitespace from default name/email","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-22T17:12:04Z","receivedAt":"2012-05-22T17:12:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 22, 2012 at 09:55:19AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > ... Any other value\n> > comes from a config file, where we will have cleaned up\n> > accidental whitespace already.\n> \n> Are you referring to the behaviour of the config parser that removes\n> leading and trailing whitespaces when reading an unquoted string value?\n\nYes, exactly.\n\n> If the user really wanted to have trailing whitespaces by quoting, we\n> would let it pass, in other words; it probably is a reasonable behaviour.\n\nRight, that's why I said \"accidental\" above; you can still do it, but I\nthink you'd have to be really trying. But I really wonder if we should\nbe blocking that, too. fmt_ident will drop it anyway, so this is really\nonly for code paths that want to use the email directly.\n\n> The same can be said about the environment variable GIT_COMMITTER_NAME and\n> friends, but \"accidental whitespaces are cleaned up already\" does not\n> apply to them.\n\nYes, but those variables don't hit this code path at all. They go\nstraight to fmt_ident, which does the cleanup. If you have code like\nthis:\n\n email = getenv(\"GIT_COMMITTER_EMAIL\");\n if (!email)\n email = ident_default_email();\n\nyou might have trim-able spaces in the getenv result, and nobody is\nhandling that (fmt_ident does, but accessing it directly does not).\nWe could go a step further and replace ident_default_email with a\ngit_committer_email() which does the above, plus any whitespace\ntrimming.\n\n> So, isn't the real rationale behind this choice to allow users who give\n> leading or trailing whitespaces in the configuration and environment\n> variables on purpose use whatever value they specified?\n\nTo be honest, the real rationale was laziness and a desire to keep the\ninterfaces simple. The risky code path (in my opinion) for getting extra\nwhitespace is pulling data from gecos, or from a file like\n/etc/mailname. IOW, my intent was to block common accidents, but not\nworry about somebody intentionally trying to push whitespace through.\n\n> I agree with the placement of trimming in this patch, but I do not quite\n> get (I do not mean \"I do not agree with\") what the quoted sentence wanted\n> to say.\n> \n> Other than that single small \"hrm...\", the entire series was a pleasant\n> read. Thanks.\n\nThanks. I'm happy to update this patch if we want it to be more\nparanoid, or I can update the commit message if it just needs explained\nbetter.\n\n-Peff\n"},{"id":"191905","messageId":"7vehqc5dxt.fsf@alter.siamese.dyndns.org","threadId":"30561","inReplyTo":"20120522171204.GB11600@sigill.intra.peff.net","subject":"Re: [PATCHv2 14/15] ident: trim whitespace from default name/email","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-22T17:21:50Z","receivedAt":"2012-05-22T17:21:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, May 22, 2012 at 09:55:19AM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > ... Any other value\n>> > comes from a config file, where we will have cleaned up\n>> > accidental whitespace already.\n>> \n>> Are you referring to the behaviour of the config parser that removes\n>> leading and trailing whitespaces when reading an unquoted string value?\n>\n> Yes, exactly.\n>\n>> If the user really wanted to have trailing whitespaces by quoting, we\n>> would let it pass, in other words; it probably is a reasonable behaviour.\n>\n> Right, that's why I said \"accidental\" above; you can still do it, but I\n> think you'd have to be really trying. But I really wonder if we should\n> be blocking that, too. fmt_ident will drop it anyway, so this is really\n> only for code paths that want to use the email directly.\n\nThanks.\n\n>> Other than that single small \"hrm...\", the entire series was a pleasant\n>> read. Thanks.\n>\n> Thanks. I'm happy to update this patch if we want it to be more\n> paranoid, or I can update the commit message if it just needs explained\n> better.\n\nNeither is necessary. I was just puzzled a bit and was worried if I was\nmissing something.\n"},{"id":"207732","messageId":"20130124232146.GA17458@google.com","threadId":"30561","inReplyTo":"20120521231017.GJ10981@sigill.intra.peff.net","subject":"[regression] Re: [PATCHv2 10/15] drop length limitations on gecos-derived names and emails","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-24T23:21:46Z","receivedAt":"2013-01-24T23:21:46Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Jeff,\n\nJeff King wrote:\n\n> When we pull the user's name from the GECOS field of the\n> passwd file (or generate an email address based on their\n> username and hostname), we put the result into a\n> static buffer.\n[...]\n> The conversion is mostly mechanical: replace string copies\n> with strbuf equivalents, and access the strbuf.buf directly.\n> There are a few exceptions:\n\nThis broke /etc/mailname handling. Before:\n\n\t$ git var GIT_COMMITTER_IDENT\n\tJonathan Nieder 1359069165 -0800\n\nAfter:\n\n\t$ git var GIT_COMMITTER_IDENT\n\tJonathan Nieder 1359069192 -0800\n\nThe cause:\n\n[...]\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -19,42 +18,27 @@ int user_ident_explicitly_given;\n[...]\n> -static int add_mailname_host(char *buf, size_t len)\n> +static int add_mailname_host(struct strbuf *buf)\n> {\n> \tFILE *mailname;\n> \n> @@ -65,7 +49,7 @@ static int add_mailname_host(char *buf, size_t len)\n> \t\t\t\tstrerror(errno));\n> \t\treturn -1;\n> \t}\n> -\tif (!fgets(buf, len, mailname)) {\n> +\tif (strbuf_getline(buf, mailname, '\\n') == EOF) {\n\nThis clears the strbuf.\n\nHow about something like this as a quick fix?\n\nReported-by: Mihai Rusu \nSigned-off-by: Jonathan Nieder \n\ndiff --git a/ident.c b/ident.c\nindex 73a06a1..cabd73f 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -41,6 +41,7 @@ static void copy_gecos(const struct passwd *w, struct strbuf *name)\n static int add_mailname_host(struct strbuf *buf)\n {\n \tFILE *mailname;\n+\tstruct strbuf mailnamebuf = STRBUF_INIT;\n \n \tmailname = fopen(\"/etc/mailname\", \"r\");\n \tif (!mailname) {\n@@ -49,14 +50,17 @@ static int add_mailname_host(struct strbuf *buf)\n \t\t\t\tstrerror(errno));\n \t\treturn -1;\n \t}\n-\tif (strbuf_getline(buf, mailname, '\\n') == EOF) {\n+\tif (strbuf_getline(&mailnamebuf, mailname, '\\n') == EOF) {\n \t\tif (ferror(mailname))\n \t\t\twarning(\"cannot read /etc/mailname: %s\",\n \t\t\t\tstrerror(errno));\n+\t\tstrbuf_release(&mailnamebuf);\n \t\tfclose(mailname);\n \t\treturn -1;\n \t}\n \t/* success! */\n+\tstrbuf_addbuf(buf, &mailnamebuf);\n+\tstrbuf_release(&mailnamebuf);\n \tfclose(mailname);\n \treturn 0;\n }\n"},{"id":"207737","messageId":"20130125010559.GA27657@sigill.intra.peff.net","threadId":"30561","inReplyTo":"20130124232146.GA17458@google.com","subject":"Re: [regression] Re: [PATCHv2 10/15] drop length limitations on gecos-derived names and emails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-25T01:05:59Z","receivedAt":"2013-01-25T01:05:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 24, 2013 at 03:21:46PM -0800, Jonathan Nieder wrote:\n\n> This broke /etc/mailname handling. Before:\n> \n> \t$ git var GIT_COMMITTER_IDENT\n> \tJonathan Nieder 1359069165 -0800\n> \n> After:\n> \n> \t$ git var GIT_COMMITTER_IDENT\n> \tJonathan Nieder 1359069192 -0800\n\nIck. I wonder how that slipped through...I know I was testing with\n/etc/mailname when developing the series, because I'm on a Debian\nsystem. We do even check this code path in t7502 (if you have the\nAUTOIDENT prereq), but of course we can't verify the actual value\nautomatically, because it could be anything. So I guess I just missed it\nduring my manual testing, and the automated testing is insufficient to\ncatch this particular breakage.\n\n> > -\tif (!fgets(buf, len, mailname)) {\n> > +\tif (strbuf_getline(buf, mailname, '\\n') == EOF) {\n> \n> This clears the strbuf.\n\nRight. Definitely the problem.\n\n> How about something like this as a quick fix?\n> \n> Reported-by: Mihai Rusu \n> Signed-off-by: Jonathan Nieder \n> \n> diff --git a/ident.c b/ident.c\n> index 73a06a1..cabd73f 100644\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -41,6 +41,7 @@ static void copy_gecos(const struct passwd *w, struct strbuf *name)\n> static int add_mailname_host(struct strbuf *buf)\n> {\n> \tFILE *mailname;\n> +\tstruct strbuf mailnamebuf = STRBUF_INIT;\n> \n> \tmailname = fopen(\"/etc/mailname\", \"r\");\n> \tif (!mailname) {\n> @@ -49,14 +50,17 @@ static int add_mailname_host(struct strbuf *buf)\n> \t\t\t\tstrerror(errno));\n> \t\treturn -1;\n> \t}\n> -\tif (strbuf_getline(buf, mailname, '\\n') == EOF) {\n> +\tif (strbuf_getline(&mailnamebuf, mailname, '\\n') == EOF) {\n> \t\tif (ferror(mailname))\n> \t\t\twarning(\"cannot read /etc/mailname: %s\",\n> \t\t\t\tstrerror(errno));\n> +\t\tstrbuf_release(&mailnamebuf);\n> \t\tfclose(mailname);\n> \t\treturn -1;\n> \t}\n> \t/* success! */\n> +\tstrbuf_addbuf(buf, &mailnamebuf);\n> +\tstrbuf_release(&mailnamebuf);\n> \tfclose(mailname);\n> \treturn 0;\n> }\n\nI think that is the only reasonable fix. Thanks for figuring it out.\n\nWe could expand the test in t7502 to check for \"@\" in the email, but it\nfeels weirdly specific to this bug. Either way,\n\nAcked-by: Jeff King \n\n(with a proper commit message, of course).\n\n-Peff\n"},{"id":"207823","messageId":"7vtxq53xt3.fsf@alter.siamese.dyndns.org","threadId":"30561","inReplyTo":"20130125010559.GA27657@sigill.intra.peff.net","subject":"Re: [regression] Re: [PATCHv2 10/15] drop length limitations on gecos-derived names and emails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-25T18:46:48Z","receivedAt":"2013-01-25T18:46:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> (with a proper commit message, of course).\n\nWill queue this one, to be merged to 'maint' and 'master'.\n\n-- >8 --\nFrom: Jonathan Nieder \nDate: Thu, 24 Jan 2013 15:21:46 -0800\nSubject: [PATCH] ident: do not drop username when reading from /etc/mailname\n\nAn earlier conversion from fgets() to strbuf_getline() in the\ncodepath to read from /etc/mailname to learn the default host-part\nof the ident e-mail address forgot that strbuf_getline() stores the\nline at the beginning of the buffer just like fgets().\n\nThe \"username@\" the caller has prepared in the strbuf, expecting the\nfunction to append the host-part to it, was lost because of this.\n\nReported-by: Mihai Rusu \nSigned-off-by: Jonathan Nieder \nAcked-by: Jeff King \nSigned-off-by: Junio C Hamano \n---\n ident.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/ident.c b/ident.c\nindex 484e0a9..fb4cc72 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -41,6 +41,7 @@ static void copy_gecos(const struct passwd *w, struct strbuf *name)\n static int add_mailname_host(struct strbuf *buf)\n {\n \tFILE *mailname;\n+\tstruct strbuf mailnamebuf = STRBUF_INIT;\n \n \tmailname = fopen(\"/etc/mailname\", \"r\");\n \tif (!mailname) {\n@@ -49,14 +50,17 @@ static int add_mailname_host(struct strbuf *buf)\n \t\t\t\tstrerror(errno));\n \t\treturn -1;\n \t}\n-\tif (strbuf_getline(buf, mailname, '\\n') == EOF) {\n+\tif (strbuf_getline(&mailnamebuf, mailname, '\\n') == EOF) {\n \t\tif (ferror(mailname))\n \t\t\twarning(\"cannot read /etc/mailname: %s\",\n \t\t\t\tstrerror(errno));\n+\t\tstrbuf_release(&mailnamebuf);\n \t\tfclose(mailname);\n \t\treturn -1;\n \t}\n \t/* success! */\n+\tstrbuf_addbuf(buf, &mailnamebuf);\n+\tstrbuf_release(&mailnamebuf);\n \tfclose(mailname);\n \treturn 0;\n }\n-- \n1.8.1.1.525.gdace530\n"},{"id":"207847","messageId":"20130125221030.GC23626@sigill.intra.peff.net","threadId":"30561","inReplyTo":"7vtxq53xt3.fsf@alter.siamese.dyndns.org","subject":"Re: [regression] Re: [PATCHv2 10/15] drop length limitations on gecos-derived names and emails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-25T22:10:30Z","receivedAt":"2013-01-25T22:10:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 25, 2013 at 10:46:48AM -0800, Junio C Hamano wrote:\n\n> Will queue this one, to be merged to 'maint' and 'master'.\n> \n> -- >8 --\n> From: Jonathan Nieder \n> Date: Thu, 24 Jan 2013 15:21:46 -0800\n> Subject: [PATCH] ident: do not drop username when reading from /etc/mailname\n\nThanks, looks fine to me (and thanks to Jonathan). One nit:\n\n> -\tif (strbuf_getline(buf, mailname, '\\n') == EOF) {\n> +\tif (strbuf_getline(&mailnamebuf, mailname, '\\n') == EOF) {\n> \t\tif (ferror(mailname))\n> \t\t\twarning(\"cannot read /etc/mailname: %s\",\n> \t\t\t\tstrerror(errno));\n> +\t\tstrbuf_release(&mailnamebuf);\n> \t\tfclose(mailname);\n> \t\treturn -1;\n> \t}\n\nThis strbuf_release is unnecessary, as an EOF return by definition means\nwe did not read anything. I don't mind it as a defensive measure,\nthough, in case the strbuf implementation changes to pre-allocate.\n\n-Peff\n"}]}