Re: [PATCH] git-mailinfo: Fix getting the subject from the body

["Lukas Sandström" <lukass@etek.chalmers.se>]

Junio C Hamano wrote:
> Lukas Sandström <lukass@etek.chalmers.se> 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@etek.chalmers.se>
>> ---
>>
>> 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 <lukass@etek.chalmers.se> 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