#1351 self-tests failing

Trunk
open
nobody
None
2
2006-11-21
2006-10-03
elsapo
No

I patched the self-test script in the subversion trunk
to run, and it reported some regressions/failures:

Failure count: 12 (of 756)

Discussion

  • elsapo
    elsapo
    2006-10-08

    Logged In: YES
    user_id=1195173

    I'm currently seeing the first failure as test#42:

    ..\..\Tools\Build\MergeDebug\diff2winmerge.exe
    ..\..\Build\MergeDebug\WinMerge.exe /noprefs /minimize /ub
    -b m/t002a.txt m/t002b.txt

    Note that it passes the unix & windows versions of the same
    test.

    Note also that I've built a patch against recent
    (post-2.8.7) diffutils to incorporate mac handling, by
    adopting the changes I did in WinMerge into the new
    diffutils. But, my revised diffutils is first failing on
    this exact same test, which goes to show, I suppose, that
    I've "correctly" merged in the latest WinMerge version
    including this bug.

     
  • Kimmo Varis
    Kimmo Varis
    2006-10-10

    Logged In: YES
    user_id=631874

    Umm.. So this adds some checks for Mac line-endings?

    Does this work? Lets see if I understand the code correctly,
    there are loops like:
    while ((c = *p++) != '\n' && (c != '\r' || *p == '\n'))
    {
    ...
    }
    which ends when c is '\n' or c + 1 is '\r' or p (next char)
    is '\n'. And c + 1 is same as p. So obviously that does not
    cover the case when c is '\r.

    So seems the added code is correctly covering the missing case.

    BUT that original code really could be written a lot more
    clearly than it is. :(

     
  • elsapo
    elsapo
    2006-10-10

    Logged In: YES
    user_id=1195173

    Your summary didn't sound correct to me.

    Let me try -- this is what I think:

    while ((c = *p++) != '\n' && (c != '\r' || *p == '\n'))

    ends when
    c is '\n'
    OR
    c is '\r' and *p is not '\n'

    Note: p[-1] is the same as c, or (I think) &c + 1 is p

    Original diffutils had lots of loops like so

    while ((c == *p++) != '\n')

    I agree this isn't really clear, but in defense of
    diffutils, it is a very popular idiom in C programming.

    Adding the logic for the other line returns to it
    really makes a mess tho -- we get this that you quoted:

    while ((c = *p++) != '\n' && (c != '\r' || *p == '\n'))

    and that is hard to grasp immediately unless you're familiar
    with it.

    Note: There were worse loops -- somewhere there is a note
    saying I deliberately unrolled one of them into separate
    statements because it is just too complicated to look at as
    a single statement.

     
  • Kimmo Varis
    Kimmo Varis
    2006-11-21

    • priority: 5 --> 2