Hi Jim, Andreas,

This commit of yours to the patch repository:

commit e0f707523cab26f74ec23f4a20a27add8702ed5b
Author: Jim Meyering <[email protected]>
Date:   Thu Feb 17 09:59:56 2011 +0100

    don't warn twice about the same invalid file name

    * src/pch.c (name_is_valid): Don't warn about the same name twice.
    * tests/bad-filenames (emit_patch): Exercise the new code.

is broken. bad[] holds pointers to the name strings, and these get freed 
when switching to the next hunk in the patch file (if I read the code 
properly...) So you end up accessing freed memory. Valgrind complains 
about that and the code will eventually fail, as was reported by an 
openSUSE user:

https://bugzilla.novell.com/show_bug.cgi?id=755136

I don't think the code even actually does what it is supposed to, as it 
can only store 2 bad names (the slots are never freed), while a given 
patch file could contain a lot more. I tried to fix it but couldn't come 
up with something easy and elegant, it seems that implementing it 
properly would need a lot more code. As this feature doesn't strike me 
as a fundamental one, I'll just revert the commit for now, and I suggest 
you do the same.

Thanks,
-- 
Jean Delvare
Suse L3

Reply via email to