Bug (!) in builderFiles2.cpp

[Ndolam]

I was writing my own diff program (and setting that as Command.DiffFiles2),
but was running into some issues.

I traced them to this bit of code in createIgnoreBlock()

      else if ( filled2 ) {
         XxLine line( XxLine::SAME, ii1, -1 );
         line.setHunkId( _curHunk );
         addLine( line );
         ++ii1;
      }
      else if ( filled1 ) {
         XxLine line( XxLine::SAME, -1, ii2 );
         line.setHunkId( _curHunk );
         addLine( line );
         ++ii2;
      }

This code says "if line2 was good, save line1 and increase the index of line1" and vice versa.
I can only assume this block of code is rarely accessed if Command.DiffFiles2 is not set.

After changing filled2 to filled1 in the first block and vice versa in the second block, things started working for me.

If I am wrong about it being a bug, I'll be both embarrassed and mystified.

[blais]

Hmm. Can you provide me with a very simple reproduction?
Say, send me two or three files I can run and witness the problem.

[Ndolam]

Try the following commands in your favourite shell:

seq 1 10 > 1-10
seq 1 11 > 1-11
xxdiff --resource 'Command.DiffFiles2: "true"' 1-10 1-11

That particular example dumps core for me, but on other examples I have tried I get a pop-up saying Something Bad Has Happened (sorry, don't remember exact text, but if it really matters I could play around).

Now, you are probably wondering why I would make the command to diff two files return success. The reason is that when I do a diff of some files I want to preprocess them and do the diff on the processed files. E.g., I want to compare two .json files for functional equality, but some people who update json files make huge formatting changes, obscuring the functional changes, so I canonicalize the two json files, and then call xxdiff on the canonicalized files. Since I have already seen the diffs, I don't want the "top level" xxdiff to do anything, and so I have my "Command.DiffFiles2" program return true. Without my fix this causes either a core dump or a pop-up, although I haven't figured out exactly when I get a core dump vs. the pop-up.

As far as I know there is no way to tell xxdiff to run some "delegate" program on the files before displaying the differences, so I did an end-run and wrote my own diff program to
(a) do on some pairs of files, and then return success
so that xxdiff does nothing else, or
(b) run diff and feed its output and exit code back to the calling xxdiff, so that
the normal output is seen in the xxdiff window.

I think (cough) that if you look at the code in createIgnoreBlock(6 args) you will think it is odd to try to read another line from input1 if
-> there was a line from input2, and
-> there was no line from input1.
Alternatively, maybe there is something going on there that I didn't understand.

Having said that, I don't really understand why
(a) we createIgnoreBlock when the diff program returns 0, and
(b) when the diff program returns non-0 this doesn't cause a problem.

I have pushed my (alleged) fix to my fork. If you feel convinced that my change does fix a bug, I'll cheerfully create the PR.

[blais]

Lots of detail, but I'm pretty convinced the way you choose to do this is making your life harder for yourself. Instead of hooking in at the diff level, write a "wrapper" program that will normalize the two files to temp files and involve diff/xxdiff on it.

…

On Mon, Nov 3, 2025 at 10:28 AM Ndolam ***@***.***> wrote: *Ndolam* left a comment (blais/xxdiff#43) <#43 (comment)> Try the following commands in your favourite shell: seq 1 10 > 1-10 seq 1 11 > 1-11 xxdiff --resource 'Command.DiffFiles2: "true"' 1-10 1-11 That particular example dumps core for me, but on other examples I have tried I get a pop-up saying *Something Bad Has Happened* (sorry, don't remember exact text, but if it really matters I could play around). Now, you are probably wondering why I would make the command to diff two files return success. The reason is that when I do a diff of some files I want to preprocess them and do the diff on the processed files. E.g., I want to compare two .json files for functional equality, but some people who update json files make huge formatting changes, obscuring the functional changes, so I canonicalize the two json files, and then call xxdiff on the canonicalized files. Since I have already seen the diffs, I don't want the "top level" xxdiff to do anything, and so I have my "Command.DiffFiles2" program return true. Without my fix this causes either a core dump or a pop-up, although I haven't figured out exactly when I get a core dump vs. the pop-up. As far as I know there is no way to tell xxdiff to run some "delegate" program on the files before displaying the differences, so I did an end-run and wrote my own diff program to (a) do on some pairs of files, and then return success so that xxdiff does nothing else, or (b) run diff and feed its output and exit code back to the calling xxdiff, so that the normal output is seen in the xxdiff window. I think (*cough*) that if you look at the code in createIgnoreBlock(6 args) you will think it is odd to try to read another line from input1 if -> there was a line from input2, and -> there was no line from input1. Alternatively, maybe there is something going on there that I didn't understand. Having said that, I don't really understand why (a) we createIgnoreBlock when the diff program returns 0, and (b) when the diff program returns non-0 this doesn't cause a problem. I have pushed my (alleged) fix to my fork. If you feel convinced that my change does fix a bug, I'll cheerfully create the PR. — Reply to this email directly, view it on GitHub <#43 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACSBEZ37VCKU6GIMWQZOCD325Q7LAVCNFSM6AAAAACK35BA3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIOBRGEYTINJXHA> . You are receiving this because you commented.Message ID: ***@***.***>

[Ndolam]

That works fine when doing diffs of two specific files. (Wrote that program many years ago. :)

The problem is when I am doing recursive diffs of directories. I could write a program that recursively descends a directory structure, but I like the way that xxdiff displays a diff of two directories and how it can do sub-dirs. I could write all that myself, but (with the change I am suggesting here) the program to do special diffs in special cases is pretty easy to write. (And now done. :-)

[blais]

I don't know what's up, now it appears to be working for me. No core dump. Can't repro.

…

On Mon, Nov 3, 2025 at 6:09 PM Ndolam ***@***.***> wrote: *Ndolam* left a comment (blais/xxdiff#43) <#43 (comment)> That works fine when doing diffs of two specific files. (Wrote that program many years ago. :) The problem is when I am doing recursive diffs of directories. I could write a program that recursively descends a directory structure, but I like the way that xxdiff displays a diff of two directories and how it can do sub-dirs. I could write all that myself, but (with the change I am suggesting here) the program to do special diffs in special cases is pretty easy to write. (And now done. :-) — Reply to this email directly, view it on GitHub <#43 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACSBEZLD4PB7AOBM56ACHL327HAHAVCNFSM6AAAAACK35BA3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIOBSHE3DSNRSGQ> . You are receiving this because you commented.Message ID: ***@***.***>

[Ndolam]

I am a github n00b, but it seems that the last PR you accepted had both my "don't skip first diff" fix, as well as my 'don't core dump' fix. So undo the change to builderFiles2.cpp in your local code, rebuild, and try again.

…

On Wed, Nov 5, 2025 at 19:30 (-0800), Martin Blais wrote: blais left a comment (blais/xxdiff#43) I don't know what's up, now it appears to be working for me. No core dump. Can't repro.