Re: [PATCH/RFC] ident: check /etc/mailname if email is unknown

[Junio C Hamano <gitster@pobox.com>]

Jonathan Nieder <jrnieder@gmail.com> writes:

> @@ -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.