Re: [PATCH/RFC] ident: check /etc/mailname if email is unknown
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 3, 2011, 05:30 UTC
- Message-ID
- <7v8vp2iqvc.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20111003045745.GA17604@elie>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 37 quoted lines
> @@ -52,6 +52,8 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)
>
> static void copy_email(const struct passwd *pw)
> {
> + FILE *mailname;
> +
> /*
> * Make up a fake email address
> * (name + '@' + hostname [+ '.' + domainname])
> @@ -61,6 +63,27 @@ static void copy_email(const struct passwd *pw)
> die("Your sysadmin must hate you!");
> memcpy(git_default_email, pw->pw_name, len);
> git_default_email[len++] = '@';
> +
> + /*
> + * The domain part comes from /etc/mailname if it is readable,
> + * or the current hostname and domain name otherwise.
> + */
> + mailname = fopen("/etc/mailname", "r");
> + if (!mailname) {
> + if (errno != ENOENT)
> + warning("cannot open /etc/mailname: %s",
> + strerror(errno));
> + } else if (fgets(git_default_email + len,
> + sizeof(git_default_email) - len, mailname)) {
> + /* success! */
> + fclose(mailname);
> + return;
> + } else {
> + if (ferror(mailname))
> + warning("cannot read /etc/mailname: %s",
> + strerror(errno));
> + fclose(mailname);
> + }
> gethostname(git_default_email + len, sizeof(git_default_email) - len);
> if (!strchr(git_default_email+len, '.')) {
> struct hostent *he = gethostbyname(git_default_email + len);I do not think this would hurt, even though I see /etc/mailname on only one of my boxes (i.e. Debian). For maintainability for the future, however, I would prefer to see the above hunk separated into a helper function to keep addition to copy_email() to the minimum, e.g.
memcpy(git_default_email, pw->pw_name, len);
git_default_email[len++] = '@';+ if (add_mailname_host(git_default_email, len, sizeof(git_default_email))) + return; /* read from "/etc/mailname" (Debian) */
gethostname(git_default_email + len, sizeof(git_default_email) - len);
...So that people who care about other distros can more easily add a single implementation to a similar location without making copy_email() too long to lose clarity. The fallback default logic that does gethostname() might also want to become a separate helper function as well.