Re: [PATCH] git mailinfo strips important context from patch subjects

From: Andreas Ericsson <ae@op5.se>
Date: Jun 29, 2009, 10:54 UTC
Message-ID: <4A489D5C.2000406@op5.se>
In-Reply-To: <4A48959A.3060404@gmail.com>

Paolo Bonzini wrote:

Show 20 quoted lines

> 
>>> case '[':
>>> if ((pos = strchr(subject->buf, ']'))) {
>>> remove = pos - subject->buf;
>>> - if (remove <= (subject->len - remove) * 2) {
>>> + if (remove <= subject->len * 2 / 3
>>> + && memmem(subject->buf, remove, 'PATCH', 5)) {
>>> strbuf_remove(subject, 0, remove + 1);
>>> continue;
>>> }
>>
>>
>> Pardon my ignorance, but wouldn't this still remove not only
>> "[PATCH 4/5]", but all of [PATCH 4/5] [sbuild]" anyway? The
>> parameters to strbuf_remove() seem unchanged.
> 
> I don't exclude I've screwed up, but note that pos is computed with 
> strchr, not strrchr.  Since the second memmem does not find [PATCH], it 
> does not remove anything.
>

It removes one character, which means the subject still gets mangled. If it *doesn't* remove one character and also doesn't break out of the loop, it'll loop indefinitely, since *subject->buf will never change.

There's something else wrong with your patch though, as mailinfo dumps core with it for a patch starting with "[PATCH] [git]". It happens in memmem(). Here's the backtrace:

(gdb) bt
#0  0x00c67c76 in memmem (haystack_start=0x8a4bae0, haystack_len=6, 
    needle_start=0x41544348, needle_len=5) at memmem.c:66
#1  0x0807a1e5 in cleanup_subject () at builtin-mailinfo.c:240
#2  handle_info () at builtin-mailinfo.c:878
#3  mailinfo () at builtin-mailinfo.c:929
#4  cmd_mailinfo (argc=4, argv=<value optimized out>, prefix=0x0)
    at builtin-mailinfo.c:966
#5  0x0804b0f7 in run_builtin () at git.c:247
#6  handle_internal_command (argc=4, argv=0xbfb00f58) at git.c:393
#7  0x0804b2e2 in run_argv () at git.c:439
#8  main (argc=4, argv=0xbfb00f58) at git.c:510

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.