git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: bs.x.ttp@recursor.net
Cc: git@vger.kernel.org
Subject: [PATCH 2/4] ident: handle NULL email when complaining of empty name
Date: Thu, 23 Feb 2017 03:13:53 -0500	[thread overview]
Message-ID: <20170223081353.doi7u77phpbpcbiw@sigill.intra.peff.net> (raw)
In-Reply-To: <20170223081157.hwfn3msfux5udmng@sigill.intra.peff.net>

If we see an empty name, we complain about and mention the
matching email in the error message (to give it some
context). However, the "email" pointer may be NULL here if
we were planning to fill it in later from ident_default_email().

This was broken by 59f929596 (fmt_ident: refactor strictness
checks, 2016-02-04). Prior to that commit, we would look up
the default name and email before doing any other actions.
So one solution would be to go back to that.

However, we can't just do so blindly. The logic for handling
the "!email" condition has grown since then. In particular,
looking up the default email can die if getpwuid() fails,
but there are other errors that should take precedence.
Commit 734c7789a (ident: check for useConfigOnly before
auto-detection of name/email, 2016-03-30) reordered the
checks so that we prefer the error message for
useConfigOnly.

Instead, we can observe that while the name-handling depends
on "email" being set, the reverse is not true. So we can
simply set up the email variable first.

This does mean that if both are bogus, we'll complain about
the email before the name. But between the two, there is no
reason to prefer one over the other.

Signed-off-by: Jeff King <peff@peff.net>
---
 ident.c                       | 26 +++++++++++++-------------
 t/t7518-ident-corner-cases.sh | 20 ++++++++++++++++++++
 2 files changed, 33 insertions(+), 13 deletions(-)
 create mode 100755 t/t7518-ident-corner-cases.sh

diff --git a/ident.c b/ident.c
index dde82983a..ea6034581 100644
--- a/ident.c
+++ b/ident.c
@@ -351,6 +351,19 @@ const char *fmt_ident(const char *name, const char *email,
 	int want_date = !(flag & IDENT_NO_DATE);
 	int want_name = !(flag & IDENT_NO_NAME);
 
+	if (!email) {
+		if (strict && ident_use_config_only
+		    && !(ident_config_given & IDENT_MAIL_GIVEN)) {
+			fputs(_(env_hint), stderr);
+			die(_("no email was given and auto-detection is disabled"));
+		}
+		email = ident_default_email();
+		if (strict && default_email_is_bogus) {
+			fputs(_(env_hint), stderr);
+			die(_("unable to auto-detect email address (got '%s')"), email);
+		}
+	}
+
 	if (want_name) {
 		int using_default = 0;
 		if (!name) {
@@ -378,19 +391,6 @@ const char *fmt_ident(const char *name, const char *email,
 		}
 	}
 
-	if (!email) {
-		if (strict && ident_use_config_only
-		    && !(ident_config_given & IDENT_MAIL_GIVEN)) {
-			fputs(_(env_hint), stderr);
-			die(_("no email was given and auto-detection is disabled"));
-		}
-		email = ident_default_email();
-		if (strict && default_email_is_bogus) {
-			fputs(_(env_hint), stderr);
-			die(_("unable to auto-detect email address (got '%s')"), email);
-		}
-	}
-
 	strbuf_reset(&ident);
 	if (want_name) {
 		strbuf_addstr_without_crud(&ident, name);
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
new file mode 100755
index 000000000..6c057afc1
--- /dev/null
+++ b/t/t7518-ident-corner-cases.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='corner cases in ident strings'
+. ./test-lib.sh
+
+# confirm that we do not segfault _and_ that we do not say "(null)", as
+# glibc systems will quietly handle our NULL pointer
+#
+# Note also that we can't use "env" here because we need to unset a variable,
+# and "-u" is not portable.
+test_expect_success 'empty name and missing email' '
+	(
+		sane_unset GIT_AUTHOR_EMAIL &&
+		GIT_AUTHOR_NAME= &&
+		test_must_fail git commit --allow-empty -m foo 2>err &&
+		test_i18ngrep ! null err
+	)
+'
+
+test_done
-- 
2.12.0.rc2.597.g959f68882


  parent reply	other threads:[~2017-02-23  8:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03  4:13 possible bug: inconsistent CLI behaviour for empty user.name bs.x.ttp
2017-02-23  8:11 ` Jeff King
2017-02-23  8:12   ` [PATCH 1/4] ident: mark error messages for translation Jeff King
2017-02-23  8:13   ` Jeff King [this message]
2017-02-23  8:15   ` [PATCH 3/4] ident: reject all-crud ident name Jeff King
2017-02-23  8:17   ` [PATCH 4/4] ident: do not ignore empty config name/email Jeff King
2017-02-23 20:58     ` Junio C Hamano
2017-02-24  1:08       ` Jeff King
2017-02-24  4:11         ` Junio C Hamano
2017-02-24  4:18           ` Jeff King
2017-02-27 15:08             ` Dennis Kaarsemaker
2017-02-27 20:42               ` Junio C Hamano
2017-02-28  5:28                 ` Christian Couder

Reply instructions:

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

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

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20170223081353.doi7u77phpbpcbiw@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=bs.x.ttp@recursor.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).