On Tue, Jun 28, 2011 at 3:57 AM, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Mon, Jun 27, 2011 at 7:25 PM, <jcorvel_at_apache.org> wrote:
>> Author: jcorvel
>> Date: Tue Jun 28 00:25:57 2011
>> New Revision: 1140388
>>
>> URL: http://svn.apache.org/viewvc?rev=1140388&view=rev
>> Log:
>> Fix a spurious failure of diff-diff3-test 2: '2-way unified diff and trivial
>> merge', reported by danielsh.
>>
>> * subversion/libsvn_diff/diff_file.c
>> (find_identical_suffix): Make sure variables had_cr and had_nl are always
>> initialized.
>>
>> Patch by: philip
>> Reported by: danielsh
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_diff/diff_file.c
>>
>> Modified: subversion/trunk/subversion/libsvn_diff/diff_file.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/diff_file.c?rev=1140388&r1=1140387&r2=1140388&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_diff/diff_file.c (original)
>> +++ subversion/trunk/subversion/libsvn_diff/diff_file.c Tue Jun 28 00:25:57 2011
>> @@ -530,7 +530,7 @@ find_identical_suffix(apr_off_t *suffix_
>> int suffix_lines_to_keep = SUFFIX_LINES_TO_KEEP;
>> svn_boolean_t is_match, reached_prefix;
>> apr_off_t lines = 0;
>> - svn_boolean_t had_cr, had_nl;
>> + svn_boolean_t had_cr, had_nl = FALSE;
>
> Contrary to your log message, this only ensures that had_nl is
> initialized: the initialization statement does not affect had_cr. For
> that you would need a second initialization statement for had_cr.
>
> (This is one reason why many of us prefer one-variable-per-line
> declaration style, rather than multiple-variables-per-line, as above.)
Oops, sorry about that. Initializing only had_nl is sufficient to fix
the issue in this case (had_cr is initialized further down in the
fuction). I will clean it up tonight (fix log message, and perform
another commit to change to one-variable-per-line), but if someone
else wants to do this: feel free.
I realize that these functions (find_identical_*) are somewhat hard to
'parse' right now, because they are so big and unwieldy. I intended to
improve them, reshuffle the code a bit, maybe split it up here and
there, but couldn't do this yet because of lack of time. And having
arrived this close to the release, I was hesitant to refactor things
because I didn't want to introduce any additional risk. Though, now
that these bugs are turning up, I kinda regret that ...
--
Johan
Received on 2011-06-28 09:53:54 CEST