Re: [bug-patch] Broken patch commit: don't warn twice about the same invalid file name

[Jim Meyering]

Andreas Grünbacher wrote:

> Jean,
>
> Am 6. April 2012 14:09 schrieb Jean Delvare <address@hidden>:
>> This commit [...] 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.
>
> Thanks. We are actually only interested in each patch here, not in all the
> patches that could be in the same input file. How about the attached change
> for a fix?
>
> Andreas
>
> From 593d7997706e523069e0f1c1a3330e9201aab442 Mon Sep 17 00:00:00 2001
> From: Andreas Gruenbacher <address@hidden>
> Date: Fri, 6 Apr 2012 14:43:33 +0200
> Subject: [PATCH] Fix use-after-free bug in name_is_valid()
>
> * src/common.h (ARRAY_SIZE): New macro.
> * src/pch.c (invalid_names): New global variable for remembering bad names.
> (intuit_diff_type): Reset invalid_names for each new patch; the names from the
> previous patch have already been freed.
> (name_is_valid): Use invalid_names.  Make the code "safer" and avoid
> duplication.

Hi Andreas,

Regardless of which patch you use,
IMHO, is is important to reference both the name of the reporter
and the bugzilla URL in the commit log.
Also useful: to include a reference like v2.6.1-115-ge0f7075
to the commit that introduced the bug. I.e.,

    Reported by Steffen Sledz in
    https://bugzilla.novell.com/show_bug.cgi?id=755136 via Jean Delvare.
    Bug introduced in commit v2.6.1-115-ge0f7075.