Dominique Pelle wrote:
> 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.
That is very good. I don't like changing things that I don't fully
understand. This sometimes happens because of time constraints...
When you figure out how to reproduce the original problem in a visible
way, we could also add it to the testset. Currently there only is
src/testdir/test47.in.
> 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.
I don't think this makes sense. The basic problem is that the "dpl"
pointer always points to entries where the idx_new values are not set.
Unless "dpl" is equal to "dp". And the entry at "dp" is only valid when
"notset" is false. I thought about checking that, but I reasoned the
condition would always be false, so I removed the !notset. But I'm not
100% sure if there isn't a leak in my reasoning.
> 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.
Basically "dp" loops through the list of existing diff entries, adding
the diffs for the third buffer. The complication is that sometimes
existing diff entries needs to be merged when they overlap. Then the
entries from "dp" to "dpl" are merged, updating the info for all the
buffers.
--
`The Guide says there is an art to flying,' said Ford, `or at least a
knack. The knack lies in learning how to throw yourself at the ground
and miss.' He smiled weakly.
-- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"
/// Bram Moolenaar -- [EMAIL PROTECTED] -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ download, build and distribute -- http://www.A-A-P.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
