To figure out the author ident for a commit, we call determine_author_info(). This function collects information from the environment, other commits (in the case of "--amend" or "-c/-C"), and the "--author" option. It then uses fmt_ident to generate the final ident string that goes into the commit object. fmt_ident is therefore responsible for any quality or validation checks on what is allowed to go into a commit. Before returning, though, we call split_ident_line on the result, and feed the individual components to hooks via the GIT_AUTHOR_* variables. Furthermore, we do extra validation by feeding the split to sane_ident_split(), which is pickier than fmt_ident (in particular, it will complain about an empty email field). If this parsing or validation fails, we skip updating the environment variables. This is bad, because it means that hooks may silently see a different ident than what we are putting into the commit. We should drop the extra sane_ident_split checks entirely, and take whatever fmt_ident has fed us (and what will go into the commit object). If parsing fails, we should actually abort here rather than continuing (and feeding the hooks bogus data). However, split_ident_line should never fail here. The ident was just generated by fmt_ident, so we know that it's sane. We can use assert_split_ident to double-check this. Note that we also teach that assertion to check that we found a date (it always should, but until now, no caller cared whether we found a date or not). Checking the return value of sane_ident_split is enough to ensure we have the name/email pointers set, and checking date_begin is enough to know that all of the date/tz variables are set. Signed-off-by: Jeff King <peff@xxxxxxxx> --- builtin/commit.c | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 2be5506..f1a9e07 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -504,7 +504,7 @@ static int is_a_merge(const struct commit *current_head) static void assert_split_ident(struct ident_split *id, const struct strbuf *buf) { - if (split_ident_line(id, buf->buf, buf->len)) + if (split_ident_line(id, buf->buf, buf->len) || !id->date_begin) die("BUG: unable to parse our own ident: %s", buf->buf); } @@ -518,20 +518,6 @@ static void export_one(const char *var, const char *s, const char *e, int hack) strbuf_release(&buf); } -static int sane_ident_split(struct ident_split *person) -{ - if (!person->name_begin || !person->name_end || - person->name_begin == person->name_end) - return 0; /* no human readable name */ - if (!person->mail_begin || !person->mail_end || - person->mail_begin == person->mail_end) - return 0; /* no usable mail */ - if (!person->date_begin || !person->date_end || - !person->tz_begin || !person->tz_end) - return 0; - return 1; -} - static int parse_force_date(const char *in, char *out, int len) { if (len < 1) @@ -606,12 +592,10 @@ static void determine_author_info(struct strbuf *author_ident) } strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT)); - if (!split_ident_line(&author, author_ident->buf, author_ident->len) && - sane_ident_split(&author)) { - export_one("GIT_AUTHOR_NAME", author.name_begin, author.name_end, 0); - export_one("GIT_AUTHOR_EMAIL", author.mail_begin, author.mail_end, 0); - export_one("GIT_AUTHOR_DATE", author.date_begin, author.tz_end, '@'); - } + assert_split_ident(&author, author_ident); + export_one("GIT_AUTHOR_NAME", author.name_begin, author.name_end, 0); + export_one("GIT_AUTHOR_EMAIL", author.mail_begin, author.mail_end, 0); + export_one("GIT_AUTHOR_DATE", author.date_begin, author.tz_end, '@'); } static int author_date_is_interesting(void) -- 2.2.0.454.g7eca6b7 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html