On 10/15/07, Bram Moolenaar <[EMAIL PROTECTED]> wrote:
> Can you verify the problem goes away with the patch I sent?
>
> I tried a few things to see any effect from the change I made. It's not
> easy to think of a situation where the previous code would fail. It
> must fail somewhere, since it's using arbitrary numbers for the third
> diff. I didn't want to spend too much time on it anyway.
There is no valgrind error with the patch indeed.
However, I would like to spend more time on it too. If
the + !notset was there, there might have been a good
reason for it (even if it's currently incorrect). With cvs
annotate, I see it was there from initial revision.
I'll try to:
- come up with a simpler test case (diffing the 3 source files
of vim is too complicated to verify whether diff is correct)
- check that the patch does not change the diff when no
errors were reported before.
- check that the patch correctly changes the diff when some
errors were reported before.
I'm wondering whether a safer way to fix it would be to
introduce a new field in the diffblock_S (or diff_T) struct,
which would tell how many elements in df_lnum[] and df_count[]
arrays are valid/initialized. These 2 arrays have same number
of elements I think, but sometimes only df_lnum/df_count[0...1]
are valid, sometimes only df_lnum/df_count[0..2] are valid and
sometimes df_lnum/df_count[0...3] are valid.
Let's call that new field dpl->initcount. In that case, code around
line 1315 could become:
diff -c -r1.27 diff.c
*** diff.c 29 Sep 2007 12:15:46 -0000 1.27
--- diff.c 15 Oct 2007 14:02:25 -0000
***************
*** 1312,1319 ****
}
for (i = idx_orig; i < idx_new + !notset; ++i)
if (curtab->tp_diffbuf[i] != NULL)
! dp->df_count[i] = dpl->df_lnum[i] + dpl->df_count[i]
! - dp->df_lnum[i] + off;
/* Delete the diff blocks that have been merged into one. */
dn = dp->df_next;
--- 1312,1320 ----
}
for (i = idx_orig; i < idx_new + !notset; ++i)
if (curtab->tp_diffbuf[i] != NULL)
! if (dpl->initcount >= i)
! dp->df_count[i] = dpl->df_lnum[i] + dpl->df_count[i]
! -
dp->df_lnum[i] + off;
/* Delete the diff blocks that have been merged into one. */
dn = dp->df_next;
This is only a partial patch (it's missing setting up dpl->initcount)
but it's enough to get the idea. That would prevent access to
uninitialised dpl->df_count[i].
But I have to admit I don't understand enough the algorithm that
populates diff blocks to say whether that's correct or not. Perhaps
it ends up being the same as removing + !notset as you did. I'll
try at least to check whether it's equivalent or not to you proposed
fix.
-- Dominique
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
