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

Paolo Bonzini wrote:

quoted

quoted

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.