From bcb2b0044b3baedf7857613ec069f300c364680c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:09:43 -0400 Subject: ident: split setup_ident into separate functions This function sets up the default name, email, and date, and is not publicly available. Let's split it into three public functions so that callers can get just the parts they need. While we're at it, let's change the interface to simple accessors. The original function was called only by fmt_ident, and contained logic for "if we already have some other value, don't load the default" which properly belongs in fmt_ident. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index 87c697c2b..0f7dcae8f 100644 --- a/ident.c +++ b/ident.c @@ -117,21 +117,20 @@ static void copy_email(const struct passwd *pw) sizeof(git_default_email) - len); } -static void setup_ident(const char **name, const char **emailp) +const char *ident_default_name(void) { - struct passwd *pw = NULL; - - /* Get the name ("gecos") */ - if (!*name && !git_default_name[0]) { - pw = getpwuid(getuid()); + if (!git_default_name[0]) { + struct passwd *pw = getpwuid(getuid()); if (!pw) die("You don't exist. Go away!"); copy_gecos(pw, git_default_name, sizeof(git_default_name)); } - if (!*name) - *name = git_default_name; + return git_default_name; +} - if (!*emailp && !git_default_email[0]) { +const char *ident_default_email(void) +{ + if (!git_default_email[0]) { const char *email = getenv("EMAIL"); if (email && email[0]) { @@ -139,19 +138,20 @@ static void setup_ident(const char **name, const char **emailp) sizeof(git_default_email)); user_ident_explicitly_given |= IDENT_MAIL_GIVEN; } else { - if (!pw) - pw = getpwuid(getuid()); + struct passwd *pw = getpwuid(getuid()); if (!pw) die("You don't exist. Go away!"); copy_email(pw); } } - if (!*emailp) - *emailp = git_default_email; + return git_default_email; +} - /* And set the default date */ +const char *ident_default_date(void) +{ if (!git_default_date[0]) datestamp(git_default_date, sizeof(git_default_date)); + return git_default_date; } static int add_raw(char *buf, size_t size, int offset, const char *str) @@ -311,7 +311,10 @@ const char *fmt_ident(const char *name, const char *email, int warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME); int name_addr_only = (flag & IDENT_NO_DATE); - setup_ident(&name, &email); + if (!name) + name = ident_default_name(); + if (!email) + email = ident_default_email(); if (!*name) { struct passwd *pw; @@ -331,7 +334,7 @@ const char *fmt_ident(const char *name, const char *email, name = git_default_name; } - strcpy(date, git_default_date); + strcpy(date, ident_default_date()); if (!name_addr_only && date_str && date_str[0]) { if (parse_date(date_str, date, sizeof(date)) < 0) die("invalid date format: %s", date_str); -- cgit v1.2.1 From 9597921b6c173d90359e2cfa4c2e36de8d1554e6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:09:54 -0400 Subject: move identity config parsing to ident.c There's no reason for this to be in config, except that once upon a time all of the config parsing was there. It makes more sense to keep the ident code together. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'ident.c') diff --git a/ident.c b/ident.c index 0f7dcae8f..bb1158f7d 100644 --- a/ident.c +++ b/ident.c @@ -388,3 +388,24 @@ int user_ident_sufficiently_given(void) return (user_ident_explicitly_given == IDENT_ALL_GIVEN); #endif } + +int git_ident_config(const char *var, const char *value, void *data) +{ + if (!strcmp(var, "user.name")) { + if (!value) + return config_error_nonbool(var); + strlcpy(git_default_name, value, sizeof(git_default_name)); + user_ident_explicitly_given |= IDENT_NAME_GIVEN; + return 0; + } + + if (!strcmp(var, "user.email")) { + if (!value) + return config_error_nonbool(var); + strlcpy(git_default_email, value, sizeof(git_default_email)); + user_ident_explicitly_given |= IDENT_MAIL_GIVEN; + return 0; + } + + return 0; +} -- cgit v1.2.1 From 2d4b4fcebdd4fb8c8cd2664b390e3bbb82370155 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:09:57 -0400 Subject: move git_default_* variables to ident.c There's no reason anybody outside of ident.c should access these directly (they should use the new accessors which make sure the variables are initialized), so we can make them file-scope statics. While we're at it, move user_ident_explicitly_given into ident.c; while still globally visible, it makes more sense to reside with the ident code. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'ident.c') diff --git a/ident.c b/ident.c index bb1158f7d..af92b2cd8 100644 --- a/ident.c +++ b/ident.c @@ -7,7 +7,11 @@ */ #include "cache.h" +#define MAX_GITNAME (1000) +static char git_default_name[MAX_GITNAME]; +static char git_default_email[MAX_GITNAME]; static char git_default_date[50]; +int user_ident_explicitly_given; #ifdef NO_GECOS_IN_PWENT #define get_gecos(ignored) "&" -- cgit v1.2.1 From 132f4b6ccb470cb209167b7806c68805ba4dc600 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:10:02 -0400 Subject: ident: trim trailing newline from /etc/mailname We use fgets to read the /etc/mailname file, which means we will typically end up with an extra newline in our git_default_email. Most of the time this doesn't matter, as fmt_ident will skip it as cruft, but there is one code path that accesses it directly (in http-push.c:lock_remote). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'ident.c') diff --git a/ident.c b/ident.c index af92b2cd8..acb3a0843 100644 --- a/ident.c +++ b/ident.c @@ -74,6 +74,10 @@ static int add_mailname_host(char *buf, size_t len) } /* success! */ fclose(mailname); + + len = strlen(buf); + if (len && buf[len-1] == '\n') + buf[len-1] = '\0'; return 0; } -- cgit v1.2.1 From b9f0ac1710ed1d2dc865ab40893720e9a242e362 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:10:11 -0400 Subject: fmt_ident: drop IDENT_WARN_ON_NO_NAME code There are no more callers who want this, so we can drop it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index acb3a0843..3b92c4465 100644 --- a/ident.c +++ b/ident.c @@ -316,7 +316,6 @@ const char *fmt_ident(const char *name, const char *email, char date[50]; int i; int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME); - int warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME); int name_addr_only = (flag & IDENT_NO_DATE); if (!name) @@ -327,13 +326,11 @@ const char *fmt_ident(const char *name, const char *email, if (!*name) { struct passwd *pw; - if ((warn_on_no_name || error_on_no_name) && - name == git_default_name && env_hint) { - fputs(env_hint, stderr); - env_hint = NULL; /* warn only once */ - } - if (error_on_no_name) + if (error_on_no_name) { + if (name == git_default_name) + fputs(env_hint, stderr); die("empty ident %s <%s> not allowed", name, email); + } pw = getpwuid(getuid()); if (!pw) die("You don't exist. Go away!"); -- cgit v1.2.1 From 060d4bb3d6c0f4ca3b85f089708b07447224bc91 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:10:14 -0400 Subject: ident: don't write fallback username into git_default_name The fmt_ident function gets a flag that tells us whether to die if the name field is blank. If it is blank and we don't die, then we fall back to the username from the passwd file. The current code writes the value into git_default_name. However, that's not necessarily correct, as the empty value might have come from git_default_name, or it might have been passed in. This leads to two potential problems: 1. If we are overriding an empty name in the passed-in value, then we may be overwriting a perfectly good name (from gitconfig or gecos) in the git_default_name buffer. Later calls to fmt_ident will end up using the fallback name, even though a better name was available. 2. If we override an empty gecos name, we end up with the fallback name in git_default_name. A later call that uses IDENT_ERROR_ON_NO_NAME will see the fallback name and think that it is a good name, instead of producing an error. In other words, a blank gecos name would cause an error with this code: git_committer_info(IDENT_ERROR_ON_NO_NAME); but not this: git_committer_info(0); git_committer_info(IDENT_ERROR_ON_NO_NAME); because in the latter case, the first call has polluted the name buffer. Instead, let's make the fallback a per-invocation variable. We can just use the pw->pw_name string directly, since it only needs to persist through the rest of the function (and we don't do any other getpwent calls). Note that while this solves (1) for future invocations of fmt_indent, the current invocation might use the fallback when it could in theory load a better value from git_default_name. However, by not passing IDENT_ERROR_ON_NO_NAME, the caller is indicating that it does not care too much about the name, anyway, so we don't bother; this is primarily about protecting future callers who do care. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index 3b92c4465..87e3cbe58 100644 --- a/ident.c +++ b/ident.c @@ -334,9 +334,7 @@ const char *fmt_ident(const char *name, const char *email, pw = getpwuid(getuid()); if (!pw) die("You don't exist. Go away!"); - strlcpy(git_default_name, pw->pw_name, - sizeof(git_default_name)); - name = git_default_name; + name = pw->pw_name; } strcpy(date, ident_default_date()); -- cgit v1.2.1 From 8587ead78ad3d2af760270b21035ffe48fde3e7b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:10:17 -0400 Subject: drop length limitations on gecos-derived names and emails When we pull the user's name from the GECOS field of the passwd file (or generate an email address based on their username and hostname), we put the result into a static buffer. While it's extremely unlikely that anybody ever hit these limits (after all, in such a case their parents must have hated them), we still had to deal with the error cases in our code. Converting these static buffers to strbufs lets us simplify the code and drop some error messages from the documentation that have confused some users. The conversion is mostly mechanical: replace string copies with strbuf equivalents, and access the strbuf.buf directly. There are a few exceptions: - copy_gecos and copy_email are the big winners in code reduction (since they no longer have to manage the string length manually) - git_ident_config wants to replace old versions of the default name (e.g., if we read the config multiple times), so it must reset+add to the strbuf instead of just adding Note that there is still one length limitation: the gethostname interface requires us to provide a static buffer, so we arbitrarily choose 1024 bytes for the hostname. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 104 ++++++++++++++++++++++++---------------------------------------- 1 file changed, 39 insertions(+), 65 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index 87e3cbe58..73a06a11f 100644 --- a/ident.c +++ b/ident.c @@ -7,9 +7,8 @@ */ #include "cache.h" -#define MAX_GITNAME (1000) -static char git_default_name[MAX_GITNAME]; -static char git_default_email[MAX_GITNAME]; +static struct strbuf git_default_name = STRBUF_INIT; +static struct strbuf git_default_email = STRBUF_INIT; static char git_default_date[50]; int user_ident_explicitly_given; @@ -19,42 +18,27 @@ int user_ident_explicitly_given; #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos) #endif -static void copy_gecos(const struct passwd *w, char *name, size_t sz) +static void copy_gecos(const struct passwd *w, struct strbuf *name) { - char *src, *dst; - size_t len, nlen; - - nlen = strlen(w->pw_name); + char *src; /* Traditionally GECOS field had office phone numbers etc, separated * with commas. Also & stands for capitalized form of the login name. */ - for (len = 0, dst = name, src = get_gecos(w); len < sz; src++) { + for (src = get_gecos(w); *src && *src != ','; src++) { int ch = *src; - if (ch != '&') { - *dst++ = ch; - if (ch == 0 || ch == ',') - break; - len++; - continue; - } - if (len + nlen < sz) { + if (ch != '&') + strbuf_addch(name, ch); + else { /* Sorry, Mr. McDonald... */ - *dst++ = toupper(*w->pw_name); - memcpy(dst, w->pw_name + 1, nlen - 1); - dst += nlen - 1; - len += nlen; + strbuf_addch(name, toupper(*w->pw_name)); + strbuf_addstr(name, w->pw_name + 1); } } - if (len < sz) - name[len] = 0; - else - die("Your parents must have hated you!"); - } -static int add_mailname_host(char *buf, size_t len) +static int add_mailname_host(struct strbuf *buf) { FILE *mailname; @@ -65,7 +49,7 @@ static int add_mailname_host(char *buf, size_t len) strerror(errno)); return -1; } - if (!fgets(buf, len, mailname)) { + if (strbuf_getline(buf, mailname, '\n') == EOF) { if (ferror(mailname)) warning("cannot read /etc/mailname: %s", strerror(errno)); @@ -74,85 +58,73 @@ static int add_mailname_host(char *buf, size_t len) } /* success! */ fclose(mailname); - - len = strlen(buf); - if (len && buf[len-1] == '\n') - buf[len-1] = '\0'; return 0; } -static void add_domainname(char *buf, size_t len) +static void add_domainname(struct strbuf *out) { + char buf[1024]; struct hostent *he; - size_t namelen; const char *domainname; - if (gethostname(buf, len)) { + if (gethostname(buf, sizeof(buf))) { warning("cannot get host name: %s", strerror(errno)); - strlcpy(buf, "(none)", len); + strbuf_addstr(out, "(none)"); return; } - namelen = strlen(buf); - if (memchr(buf, '.', namelen)) + strbuf_addstr(out, buf); + if (strchr(buf, '.')) return; he = gethostbyname(buf); - buf[namelen++] = '.'; - buf += namelen; - len -= namelen; + strbuf_addch(out, '.'); if (he && (domainname = strchr(he->h_name, '.'))) - strlcpy(buf, domainname + 1, len); + strbuf_addstr(out, domainname + 1); else - strlcpy(buf, "(none)", len); + strbuf_addstr(out, "(none)"); } -static void copy_email(const struct passwd *pw) +static void copy_email(const struct passwd *pw, struct strbuf *email) { /* * Make up a fake email address * (name + '@' + hostname [+ '.' + domainname]) */ - size_t len = strlen(pw->pw_name); - if (len > sizeof(git_default_email)/2) - die("Your sysadmin must hate you!"); - memcpy(git_default_email, pw->pw_name, len); - git_default_email[len++] = '@'; - - if (!add_mailname_host(git_default_email + len, - sizeof(git_default_email) - len)) + strbuf_addstr(email, pw->pw_name); + strbuf_addch(email, '@'); + + if (!add_mailname_host(email)) return; /* read from "/etc/mailname" (Debian) */ - add_domainname(git_default_email + len, - sizeof(git_default_email) - len); + add_domainname(email); } const char *ident_default_name(void) { - if (!git_default_name[0]) { + if (!git_default_name.len) { struct passwd *pw = getpwuid(getuid()); if (!pw) die("You don't exist. Go away!"); - copy_gecos(pw, git_default_name, sizeof(git_default_name)); + copy_gecos(pw, &git_default_name); } - return git_default_name; + return git_default_name.buf; } const char *ident_default_email(void) { - if (!git_default_email[0]) { + if (!git_default_email.len) { const char *email = getenv("EMAIL"); if (email && email[0]) { - strlcpy(git_default_email, email, - sizeof(git_default_email)); + strbuf_addstr(&git_default_email, email); user_ident_explicitly_given |= IDENT_MAIL_GIVEN; } else { struct passwd *pw = getpwuid(getuid()); if (!pw) die("You don't exist. Go away!"); - copy_email(pw); + copy_email(pw, &git_default_email); } } - return git_default_email; + return git_default_email.buf; } const char *ident_default_date(void) @@ -327,7 +299,7 @@ const char *fmt_ident(const char *name, const char *email, struct passwd *pw; if (error_on_no_name) { - if (name == git_default_name) + if (name == git_default_name.buf) fputs(env_hint, stderr); die("empty ident %s <%s> not allowed", name, email); } @@ -397,7 +369,8 @@ int git_ident_config(const char *var, const char *value, void *data) if (!strcmp(var, "user.name")) { if (!value) return config_error_nonbool(var); - strlcpy(git_default_name, value, sizeof(git_default_name)); + strbuf_reset(&git_default_name); + strbuf_addstr(&git_default_name, value); user_ident_explicitly_given |= IDENT_NAME_GIVEN; return 0; } @@ -405,7 +378,8 @@ int git_ident_config(const char *var, const char *value, void *data) if (!strcmp(var, "user.email")) { if (!value) return config_error_nonbool(var); - strlcpy(git_default_email, value, sizeof(git_default_email)); + strbuf_reset(&git_default_email); + strbuf_addstr(&git_default_email, value); user_ident_explicitly_given |= IDENT_MAIL_GIVEN; return 0; } -- cgit v1.2.1 From 2f70587502b6b5a8cfda5a3eff54392f48e2db8d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:10:20 -0400 Subject: ident: report passwd errors with a more friendly message When getpwuid fails, we give a cute but cryptic message. While it makes sense if you know that getpwuid or identity functions are being called, this code is triggered behind the scenes by quite a few git commands these days (e.g., receive-pack on a remote server might use it for a reflog; the current message is hard to distinguish from an authentication error). Let's switch to something that gives a little more context. While we're at it, we can factor out all of the cut-and-pastes of the "you don't exist" message into a wrapper function. Rather than provide xgetpwuid, let's make it even more specific to just getting the passwd entry for the current uid. That's the only way we use getpwuid anyway, and it lets us make an even more specific error message. The current message also fails to mention errno. While the usual cause for getpwuid failing is that the user does not exist, mentioning errno makes it easier to diagnose these problems. Note that POSIX specifies that errno remain untouched if the passwd entry does not exist (but will be set on actual errors), whereas some systems will return ENOENT or similar for a missing entry. We handle both cases in our wrapper. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index 73a06a11f..5aec073b9 100644 --- a/ident.c +++ b/ident.c @@ -100,12 +100,8 @@ static void copy_email(const struct passwd *pw, struct strbuf *email) const char *ident_default_name(void) { - if (!git_default_name.len) { - struct passwd *pw = getpwuid(getuid()); - if (!pw) - die("You don't exist. Go away!"); - copy_gecos(pw, &git_default_name); - } + if (!git_default_name.len) + copy_gecos(xgetpwuid_self(), &git_default_name); return git_default_name.buf; } @@ -117,12 +113,8 @@ const char *ident_default_email(void) if (email && email[0]) { strbuf_addstr(&git_default_email, email); user_ident_explicitly_given |= IDENT_MAIL_GIVEN; - } else { - struct passwd *pw = getpwuid(getuid()); - if (!pw) - die("You don't exist. Go away!"); - copy_email(pw, &git_default_email); - } + } else + copy_email(xgetpwuid_self(), &git_default_email); } return git_default_email.buf; } @@ -303,9 +295,7 @@ const char *fmt_ident(const char *name, const char *email, fputs(env_hint, stderr); die("empty ident %s <%s> not allowed", name, email); } - pw = getpwuid(getuid()); - if (!pw) - die("You don't exist. Go away!"); + pw = xgetpwuid_self(); name = pw->pw_name; } -- cgit v1.2.1 From f8254d321cc42a6d47abf79b0f93d16e28432df3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:10:23 -0400 Subject: ident: use full dns names to generate email addresses When we construct an email address from the username and hostname, we generate the host part of the email with this procedure: 1. add the result of gethostname 2. if it has a dot, ok, it's fully qualified 3. if not, then look up the unqualified hostname via gethostbyname; take the domain name of the result and append it to the hostname Step 3 can actually produce a bogus result, as the name returned by gethostbyname may not be related to the hostname we fed it (e.g., consider a machine "foo" with names "foo.one.example.com" and "bar.two.example.com"; we may have the latter returned and generate the bogus name "foo.two.example.com"). This patch simply uses the full hostname returned by gethostbyname. In the common case that the first part is the same as the unqualified hostname, the behavior is identical. And in the case that it is not the same, we are much more likely to be generating a valid name. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index 5aec073b9..b111e3414 100644 --- a/ident.c +++ b/ident.c @@ -65,23 +65,18 @@ static void add_domainname(struct strbuf *out) { char buf[1024]; struct hostent *he; - const char *domainname; if (gethostname(buf, sizeof(buf))) { warning("cannot get host name: %s", strerror(errno)); strbuf_addstr(out, "(none)"); return; } - strbuf_addstr(out, buf); if (strchr(buf, '.')) - return; - - he = gethostbyname(buf); - strbuf_addch(out, '.'); - if (he && (domainname = strchr(he->h_name, '.'))) - strbuf_addstr(out, domainname + 1); + strbuf_addstr(out, buf); + else if ((he = gethostbyname(buf)) && strchr(he->h_name, '.')) + strbuf_addstr(out, he->h_name); else - strbuf_addstr(out, "(none)"); + strbuf_addf(out, "%s.(none)", buf); } static void copy_email(const struct passwd *pw, struct strbuf *email) -- cgit v1.2.1 From c96f0c8d0a9f1eeabb2ea49cb7ede954a64bd540 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:10:26 -0400 Subject: ident: use a dynamic strbuf in fmt_ident Now that we accept arbitrary-sized names and email addresses, the only remaining limit is in the actual formatting of the names into a buffer. The current limit is 1000 characters, which is not likely to be reached, but using a strbuf is one less error condition we have to worry about. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 43 +++++++++++++++---------------------------- 1 file changed, 15 insertions(+), 28 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index b111e3414..cefb8294c 100644 --- a/ident.c +++ b/ident.c @@ -121,15 +121,6 @@ const char *ident_default_date(void) return git_default_date; } -static int add_raw(char *buf, size_t size, int offset, const char *str) -{ - size_t len = strlen(str); - if (offset + len > size) - return size; - memcpy(buf + offset, str, len); - return offset + len; -} - static int crud(unsigned char c) { return c <= 32 || @@ -148,7 +139,7 @@ static int crud(unsigned char c) * Copy over a string to the destination, but avoid special * characters ('\n', '<' and '>') and remove crud at the end */ -static int copy(char *buf, size_t size, int offset, const char *src) +static void strbuf_addstr_without_crud(struct strbuf *sb, const char *src) { size_t i, len; unsigned char c; @@ -172,19 +163,19 @@ static int copy(char *buf, size_t size, int offset, const char *src) /* * Copy the rest to the buffer, but avoid the special * characters '\n' '<' and '>' that act as delimiters on - * an identification line + * an identification line. We can only remove crud, never add it, + * so 'len' is our maximum. */ + strbuf_grow(sb, len); for (i = 0; i < len; i++) { c = *src++; switch (c) { case '\n': case '<': case '>': continue; } - if (offset >= size) - return size; - buf[offset++] = c; + sb->buf[sb->len++] = c; } - return offset; + sb->buf[sb->len] = '\0'; } /* @@ -271,9 +262,8 @@ static const char *env_hint = const char *fmt_ident(const char *name, const char *email, const char *date_str, int flag) { - static char buffer[1000]; + static struct strbuf ident = STRBUF_INIT; char date[50]; - int i; int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME); int name_addr_only = (flag & IDENT_NO_DATE); @@ -300,19 +290,16 @@ const char *fmt_ident(const char *name, const char *email, die("invalid date format: %s", date_str); } - i = copy(buffer, sizeof(buffer), 0, name); - i = add_raw(buffer, sizeof(buffer), i, " <"); - i = copy(buffer, sizeof(buffer), i, email); + strbuf_reset(&ident); + strbuf_addstr_without_crud(&ident, name); + strbuf_addstr(&ident, " <"); + strbuf_addstr_without_crud(&ident, email); + strbuf_addch(&ident, '>'); if (!name_addr_only) { - i = add_raw(buffer, sizeof(buffer), i, "> "); - i = copy(buffer, sizeof(buffer), i, date); - } else { - i = add_raw(buffer, sizeof(buffer), i, ">"); + strbuf_addch(&ident, ' '); + strbuf_addstr_without_crud(&ident, date); } - if (i >= sizeof(buffer)) - die("Impossibly long personal identifier"); - buffer[i] = 0; - return buffer; + return ident.buf; } const char *fmt_name(const char *name, const char *email) -- cgit v1.2.1 From be641abdb544f00adb7ae6fdab41f9bd5453e206 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 21 May 2012 19:10:29 -0400 Subject: ident: trim whitespace from default name/email Usually these values get fed to fmt_ident, which will trim any cruft anyway, but there are a few code paths which use them directly. Let's clean them up for the benefit of those callers. Furthermore, fmt_ident will look at the pre-trimmed value and decide whether to invoke ERROR_ON_NO_NAME; this check can be fooled by a name consisting only of spaces. Note that we only bother to clean up when we are pulling the information from gecos or from system files. Any other value comes from a config file, where we will have cleaned up accidental whitespace already. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index cefb8294c..e279039ac 100644 --- a/ident.c +++ b/ident.c @@ -95,8 +95,10 @@ static void copy_email(const struct passwd *pw, struct strbuf *email) const char *ident_default_name(void) { - if (!git_default_name.len) + if (!git_default_name.len) { copy_gecos(xgetpwuid_self(), &git_default_name); + strbuf_trim(&git_default_name); + } return git_default_name.buf; } @@ -110,6 +112,7 @@ const char *ident_default_email(void) user_ident_explicitly_given |= IDENT_MAIL_GIVEN; } else copy_email(xgetpwuid_self(), &git_default_email); + strbuf_trim(&git_default_email); } return git_default_email.buf; } -- cgit v1.2.1 From b00f6cfcd7232d90c4625c42eb9694d4ed2dc615 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 May 2012 19:26:32 -0400 Subject: ident: reword empty ident error message There's on point in printing the name, since it is by definition the empty string if we have reached this code path. Instead, let's be more clear that we are complaining about the empty name, but still show the email address that it is attached to (since that may provide some context to the user). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index e279039ac..f5160e1f4 100644 --- a/ident.c +++ b/ident.c @@ -281,7 +281,7 @@ const char *fmt_ident(const char *name, const char *email, if (error_on_no_name) { if (name == git_default_name.buf) fputs(env_hint, stderr); - die("empty ident %s <%s> not allowed", name, email); + die("empty ident name (for <%s>) not allowed", email); } pw = xgetpwuid_self(); name = pw->pw_name; -- cgit v1.2.1 From 359b27add341878a13c6a8b85849b75b78246c7e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 May 2012 19:26:50 -0400 Subject: ident: refactor NO_DATE flag in fmt_ident As a short-hand, we extract this flag into the local variable "name_addr_only". It's more accurate to simply negate this and refer to it as "want_date", which will be less confusing when we add more NO_* flags. While we're touching this part of the code, let's move the call to ident_default_date() only when we are actually going to use it, not when we have NO_DATE set, or when we get a date from the environment. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index f5160e1f4..59beef2f3 100644 --- a/ident.c +++ b/ident.c @@ -268,7 +268,7 @@ const char *fmt_ident(const char *name, const char *email, static struct strbuf ident = STRBUF_INIT; char date[50]; int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME); - int name_addr_only = (flag & IDENT_NO_DATE); + int want_date = !(flag & IDENT_NO_DATE); if (!name) name = ident_default_name(); @@ -287,10 +287,13 @@ const char *fmt_ident(const char *name, const char *email, name = pw->pw_name; } - strcpy(date, ident_default_date()); - if (!name_addr_only && date_str && date_str[0]) { - if (parse_date(date_str, date, sizeof(date)) < 0) - die("invalid date format: %s", date_str); + if (want_date) { + if (date_str && date_str[0]) { + if (parse_date(date_str, date, sizeof(date)) < 0) + die("invalid date format: %s", date_str); + } + else + strcpy(date, ident_default_date()); } strbuf_reset(&ident); @@ -298,7 +301,7 @@ const char *fmt_ident(const char *name, const char *email, strbuf_addstr(&ident, " <"); strbuf_addstr_without_crud(&ident, email); strbuf_addch(&ident, '>'); - if (!name_addr_only) { + if (want_date) { strbuf_addch(&ident, ' '); strbuf_addstr_without_crud(&ident, date); } -- cgit v1.2.1 From c15e1987aec562107bd8e4a9fdeebf6d27d0e53a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 May 2012 19:27:24 -0400 Subject: ident: let callers omit name with fmt_indent Most callers want to see all of "$name <$email> $date", but a few want only limited parts, omitting the date, or even the name. We already have IDENT_NO_DATE to handle the date part, but there's not a good option for getting just the email. Callers have to done one of: 1. Call ident_default_email; this does not respect environment variables, nor does it promise to trim whitespace or other crud from the result. 2. Call git_{committer,author}_info; this returns the name and email, leaving the caller to parse out the wanted bits. This patch adds IDENT_NO_NAME; it stops short of adding IDENT_NO_EMAIL, as no callers want it (nor are likely to), and it complicates the error handling of the function. When no name is requested, the angle brackets (<>) around the email address are also omitted. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index 59beef2f3..8b5080dfe 100644 --- a/ident.c +++ b/ident.c @@ -269,13 +269,14 @@ const char *fmt_ident(const char *name, const char *email, char date[50]; int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME); int want_date = !(flag & IDENT_NO_DATE); + int want_name = !(flag & IDENT_NO_NAME); - if (!name) + if (want_name && !name) name = ident_default_name(); if (!email) email = ident_default_email(); - if (!*name) { + if (want_name && !*name) { struct passwd *pw; if (error_on_no_name) { @@ -297,10 +298,13 @@ const char *fmt_ident(const char *name, const char *email, } strbuf_reset(&ident); - strbuf_addstr_without_crud(&ident, name); - strbuf_addstr(&ident, " <"); + if (want_name) { + strbuf_addstr_without_crud(&ident, name); + strbuf_addstr(&ident, " <"); + } strbuf_addstr_without_crud(&ident, email); - strbuf_addch(&ident, '>'); + if (want_name) + strbuf_addch(&ident, '>'); if (want_date) { strbuf_addch(&ident, ' '); strbuf_addstr_without_crud(&ident, date); -- cgit v1.2.1 From f9bc573fdaeaf8621008f3f49aaaa64869791691 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 May 2012 19:28:40 -0400 Subject: ident: rename IDENT_ERROR_ON_NO_NAME to IDENT_STRICT Callers who ask for ERROR_ON_NO_NAME are not so much concerned that the name will be blank (because, after all, we will fall back to using the username), but rather it is a check to make sure that low-quality identities do not end up in things like commit messages or emails (whereas it is OK for them to end up in things like reflogs). When future commits add more quality checks on the identity, each of these callers would want to use those checks, too. Rather than modify each of them later to add a new flag, let's refactor the flag. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'ident.c') diff --git a/ident.c b/ident.c index 8b5080dfe..c42258f4e 100644 --- a/ident.c +++ b/ident.c @@ -267,7 +267,7 @@ const char *fmt_ident(const char *name, const char *email, { static struct strbuf ident = STRBUF_INIT; char date[50]; - int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME); + int strict = (flag & IDENT_STRICT); int want_date = !(flag & IDENT_NO_DATE); int want_name = !(flag & IDENT_NO_NAME); @@ -279,7 +279,7 @@ const char *fmt_ident(const char *name, const char *email, if (want_name && !*name) { struct passwd *pw; - if (error_on_no_name) { + if (strict) { if (name == git_default_name.buf) fputs(env_hint, stderr); die("empty ident name (for <%s>) not allowed", email); @@ -314,7 +314,7 @@ const char *fmt_ident(const char *name, const char *email, const char *fmt_name(const char *name, const char *email) { - return fmt_ident(name, email, NULL, IDENT_ERROR_ON_NO_NAME | IDENT_NO_DATE); + return fmt_ident(name, email, NULL, IDENT_STRICT | IDENT_NO_DATE); } const char *git_author_info(int flag) -- cgit v1.2.1 From 8c5b1ae1b26a7512eb29e75391b8b24c0d0439e7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 24 May 2012 19:32:37 -0400 Subject: ident: reject bogus email addresses with IDENT_STRICT If we come up with a hostname like "foo.(none)" because the user's machine is not fully qualified, we should reject this in strict mode (e.g., when we are making a commit object), just as we reject an empty gecos username. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- ident.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'ident.c') diff --git a/ident.c b/ident.c index c42258f4e..98852c7cc 100644 --- a/ident.c +++ b/ident.c @@ -288,6 +288,12 @@ const char *fmt_ident(const char *name, const char *email, name = pw->pw_name; } + if (strict && email == git_default_email.buf && + strstr(email, "(none)")) { + fputs(env_hint, stderr); + die("unable to auto-detect email address (got '%s')", email); + } + if (want_date) { if (date_str && date_str[0]) { if (parse_date(date_str, date, sizeof(date)) < 0) -- cgit v1.2.1