From: Lukas Sandström Date: Sat, 12 Jul 2008 21:45:30 GMT Subject: Re: [PATCH] git-mailinfo: Fix getting the subject from the body Message-ID: <487925FA.5020001@etek.chalmers.se> In-Reply-To: <7v3amfxx3a.fsf@gitster.siamese.dyndns.org> Junio C Hamano wrote: > Lukas Sandström 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 >> --- >> >> 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. Right, I noticed that too. It's fixed in the strbuf conversion, I think. Lukas Sandström wrote: > After looking at this part some more, I see that there is no guarantee > that hdr_data[i] != NULL in this codepath, and then we won't use the > subject anyway. I'll be hiking the next week, in case you wonder why I'm not responding. I'll try to get another version of the patches out before I leave. /Lukas