Lukas Sandström <lukass@xxxxxxxxxxxxxxxx> writes:
> "Subject: " isn't in the static array "header", and thus
> memcmp("Subject: ", header[i], 7) will never match.
>
> Signed-off-by: Lukas Sandström <lukass@xxxxxxxxxxxxxxxx>
> ---
>
> This has been broken since 2007-03-12, with commit
> 87ab799234639c26ea10de74782fa511cb3ca606
> so it might not be very important.
>
> builtin-mailinfo.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
> index 962aa34..2d1520f 100644
> --- a/builtin-mailinfo.c
> +++ b/builtin-mailinfo.c
> @@ -334,7 +334,7 @@ static int check_header(char *line, unsigned linesize, char **hdr_data, int over
> return 1;
> if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
> for (i = 0; header[i]; i++) {
> - if (!memcmp("Subject: ", header[i], 9)) {
> + if (!memcmp("Subject", header[i], 7)) {
> if (! handle_header(line, hdr_data[i], 0)) {
> return 1;
> }
Actually, I do not think your patch alone makes any difference, and the
original code looks somewhat bogus. If there is no "Subject: " in the
same section of the message (either in e-mail header in which case
hdr_data == p_hdr_data[], or in the message body part in which case
hdr_data == s_hdr_data[]), hdr_data[1] will be NULL, because the only
place that allocates the storage for the data is the first loop of this
function that deals with real-RFC2822-header-looking lines.
You'd probably need something like this on top of your patch to actually
activate the code.
Another thing I noticed and found puzzling is the handling of ">From "
line that is shown in the context below. check_header() is supposed to
return true when it handled header (i.e. not part of the commit message)
and return false when line is not part of the header. As ">From " is part
of the commit log message, shouldn't it return zero?
Don, this part was what you introduced. Has this codepath ever been
exercised in the real life?
builtin-mailinfo.c | 2 ++
t/t5100-mailinfo.sh | 2 +-
t/t5100/sample.mbox | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 2d1520f..13f0502 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -332,12 +332,14 @@ static int check_header(char *line, unsigned linesize, char **hdr_data, int over
/* for inbody stuff */
if (!memcmp(">From", line, 5) && isspace(line[5]))
return 1;
if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
for (i = 0; header[i]; i++) {
if (!memcmp("Subject", header[i], 7)) {
+ if (!hdr_data[i])
+ hdr_data[i] = xmalloc(linesize + 20);
if (! handle_header(line, hdr_data[i], 0)) {
return 1;
}
}
}
}
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 2d1520f..13f0502 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -335,6 +335,8 @@ static int check_header(char *line, unsigned linesize, char **hdr_data, int over
if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
for (i = 0; header[i]; i++) {
if (!memcmp("Subject", header[i], 7)) {
+ if (!hdr_data[i])
+ hdr_data[i] = xmalloc(linesize + 20);
if (! handle_header(line, hdr_data[i], 0)) {
return 1;
}
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 577ecc2..e9f3e72 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
'git mailsplit -o. ../t5100/sample.mbox >last &&
last=`cat last` &&
echo total is $last &&
- test `cat last` = 9'
+ test `cat last` = 10'
for mail in `echo 00*`
do
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 0476b96..aba57f9 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -430,3 +430,38 @@ index b426a14..97756ec 100644
=20
=20
2. When the environment variable 'GIT_EXTERNAL_DIFF' is set, the
+From b9704a518e21158433baa2cc2d591fea687967f6 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Lukas=20Sandstr=C3=B6m?= <lukass@xxxxxxxxxxxxxxxx>
+Date: Thu, 10 Jul 2008 23:41:33 +0200
+Subject: Re: discussion that lead to this patch
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+[PATCH] git-mailinfo: Fix getting the subject from the body
+
+"Subject: " isn't in the static array "header", and thus
+memcmp("Subject: ", header[i], 7) will never match.
+
+Signed-off-by: Lukas Sandström <lukass@xxxxxxxxxxxxxxxx>
+Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
+---
+ builtin-mailinfo.c | 2 +-
+ 1 files changed, 1 insertions(+), 1 deletions(-)
+
+diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
+index 962aa34..2d1520f 100644
+--- a/builtin-mailinfo.c
++++ b/builtin-mailinfo.c
+@@ -334,7 +334,7 @@ static int check_header(char *line, unsigned linesize, char **hdr_data, int over
+ return 1;
+ if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
+ for (i = 0; header[i]; i++) {
+- if (!memcmp("Subject: ", header[i], 9)) {
++ if (!memcmp("Subject", header[i], 7)) {
+ if (! handle_header(line, hdr_data[i], 0)) {
+ return 1;
+ }
+--
+1.5.6.2.455.g1efb2
+
--
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
