r29665 causing issue with piped diff

[Daniel Sahlberg]

Hi,

As noted in the TortoiseSVN group [1], TortoiseUDiff will quit with an error message "The pipe has been ended" if you try to pipe a diff to the program. The original post uses git but it can be reproduced with any source (for example piping from svn).

The root cause seems to be the changes by csware in r29665 on December 18th (2023):

[[[

TortoiseUDiff: Improve error reporting

Based on TortoiseGit rev. 0cadd137babbbd87ba834020b101d05d21b7e8e4.
]]]

The code (only relevant lines):

[[[

    BOOL bRet  = ReadFile(hFile, data, sizeof(data), &dwRead, nullptr);
    while ((dwRead > 0) && (bRet))
    {

... send data to Scintilla and check status ...
        bRet = ReadFile(hFile, data, sizeof(data), &dwRead, nullptr);
    }
    if (!bRet)
    {
        if (hFile || wantStdIn)
        {
            MessageBox(*this, static_cast<LPCWSTR>(CFormatMessageWrapper()), L"TortoiseUDiff", MB_ICONEXCLAMATION);
            return false;
        }
...

     }

]]]

What seems to happen is that ReadFile returns when there is no more data. This causes MessageBox to be called with GetLastError() being ERROR_BROKEN_PIPE.

I think the same would happen with TortoiseGitUDiff (see [2]).

I'm not sure I understand exactly what is supposed to happen, in particular the check for if (hFile || wantStdIn). wantStdIn is set if TortoiseUDiff is called with /b, but documentation says /b is optional and STDIN will be used by default.

Maybe a check for GetLastError() == ERROR_BROKEN_PIPE? I can't figure out from the documentation for ReadFile [4] how to detect EOF for a pipe.

Kind regards

Daniel

[1] https://groups.google.com/g/tortoisesvn/c/oyDNJ4oxPDA

[2] https://gitlab.com/tortoisegit/tortoisegit/-/blob/0cadd137babbbd87ba834020b101d05d21b7e8e4/src/TortoiseUDiff/MainWindow.cpp#L801

[3] https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-

[4] https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile?devlangs=cpp

[Stefan]

On Wednesday, July 3, 2024 at 3:40:54 PM UTC+2 daniel.l...@gmail.com wrote:

Hi,

As noted in the TortoiseSVN group [1], TortoiseUDiff will quit with an error message "The pipe has been ended" if you try to pipe a diff to the program. The original post uses git but it can be reproduced with any source (for example piping from svn).

The root cause seems to be the changes by csware in r29665 on December 18th (2023):

[[[

TortoiseUDiff: Improve error reporting

Based on TortoiseGit rev. 0cadd137babbbd87ba834020b101d05d21b7e8e4.
]]]

The code (only relevant lines):

[[[

    BOOL bRet  = ReadFile(hFile, data, sizeof(data), &dwRead, nullptr);
    while ((dwRead > 0) && (bRet))
    {

... send data to Scintilla and check status ...
        bRet = ReadFile(hFile, data, sizeof(data), &dwRead, nullptr);
    }
    if (!bRet)
    {
        if (hFile || wantStdIn)
        {
            MessageBox(*this, static_cast<LPCWSTR>(CFormatMessageWrapper()), L"TortoiseUDiff", MB_ICONEXCLAMATION);
            return false;
        }
...

     }

]]]

What seems to happen is that ReadFile returns when there is no more data. This causes MessageBox to be called with GetLastError() being ERROR_BROKEN_PIPE.

I think the same would happen with TortoiseGitUDiff (see [2]).

I'm not sure I understand exactly what is supposed to happen, in particular the check for if (hFile || wantStdIn). wantStdIn is set if TortoiseUDiff is called with /b, but documentation says /b is optional and STDIN will be used by default.

I think 'wantStdIn' is used to show errors in case stdin is not available. Because in that case, hFile would be null and no error would be reported.

 

Maybe a check for GetLastError() == ERROR_BROKEN_PIPE? I can't figure out from the documentation for ReadFile [4] how to detect EOF for a pipe.

Yes, that's the correct solution. Because when piping, the pipe gets broken when the source process (e.g. git.exe when doing a 'git diff | tortoiseudiff.exe') ends.

I'll commit a fix soon.

Stefan

[Daniel Sahlberg]

r29700 works fine. Thanks!

Kind regards,

Daniel