All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org, computersforpeace@gmail.com
Cc: Jonathan Tan <jonathantanmy@google.com>
Subject: [PATCH] sequencer: require trailing NL in footers
Date: Tue, 25 Apr 2017 12:06:51 -0700	[thread overview]
Message-ID: <20170425190651.8910-1-jonathantanmy@google.com> (raw)
In-Reply-To: <20170421220155.GA142345@google.com>

In commit 967dfd4 ("sequencer: use trailer's trailer layout",
2016-11-29), sequencer was taught to use the same mechanism as
interpret-trailers to determine the nature of the trailer of a commit
message (referred to as the "footer" in sequencer.c). However, the
requirement that a footer end in a newline character was inadvertently
removed. Restore that requirement.

While writing this commit, I noticed that if the "ignore_footer"
parameter in "has_conforming_footer" is greater than the distance
between the trailer start and sb->len, "has_conforming_footer" will
return an unexpected result. This does not occur in practice, because
"ignore_footer" is either zero or the return value of an invocation to
"ignore_non_trailer", which only skips empty lines and comment lines.
This commit contains a comment explaining this in the function's
documentation.

Reported-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---

Thanks for the bug report. Here's a fix - I've verified this with the
way to reproduce provided in the original e-mail, and it seems to work
now.

The commit message of the referenced commit
(7b309aef0463340d3ad5449d1f605d14e10a4225) does not end in a newline,
which is probably why different behavior was observed with this commit
(as compared to others).

 sequencer.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 77afecaeb..4cbe64b7f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -151,6 +151,12 @@ static const char *get_todo_path(const struct replay_opts *opts)
  * Returns 1 for conforming footer
  * Returns 2 when sob exists within conforming footer
  * Returns 3 when sob exists within conforming footer as last entry
+ *
+ * A footer that does not end in a newline is considered non-conforming.
+ *
+ * ignore_footer, if not zero, should be the return value of an invocation to
+ * ignore_non_trailer. See the documentation of that function for more
+ * information.
  */
 static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
 	int ignore_footer)
@@ -159,6 +165,11 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
 	int i;
 	int found_sob = 0, found_sob_last = 0;
 
+	if (sb->len <= ignore_footer)
+		return 0;
+	if (sb->buf[sb->len - ignore_footer - 1] != '\n')
+		return 0;
+
 	trailer_info_get(&info, sb->buf);
 
 	if (info.trailer_start == info.trailer_end)
-- 
2.13.0.rc0.306.g87b477812d-goog


  parent reply	other threads:[~2017-04-25 19:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-21 22:01 [GIT 2.12.2 REGRESSION] git cherry-pick -x Brian Norris
2017-04-21 22:10 ` Stefan Beller
2017-04-21 22:13   ` Jeff King
2017-04-25 19:06 ` Jonathan Tan [this message]
2017-04-25 21:04   ` [PATCH] sequencer: require trailing NL in footers Stefan Beller
2017-04-25 21:51     ` Jonathan Tan
2017-04-25 21:56       ` Stefan Beller
2017-04-25 22:05         ` Johannes Schindelin
2017-04-25 23:39           ` Jonathan Nieder
2017-04-25 22:30         ` Brian Norris
2017-04-26 20:31           ` Brian Norris
2017-04-25 21:56   ` Johannes Schindelin
2017-04-25 23:34   ` Jonathan Nieder
2017-04-25 23:57   ` [PATCH v2] " Jonathan Tan
2017-04-26  0:07     ` Jonathan Nieder
2017-04-26  2:08       ` Junio C Hamano
2017-04-26  9:09         ` Johannes Schindelin
2017-04-26 20:50   ` [PATCH v3] sequencer: add newline before adding footers Jonathan Tan

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

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

  git send-email \
    --in-reply-to=20170425190651.8910-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=computersforpeace@gmail.com \
    --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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.