Re: r29665 causing issue with piped diff

Stefan via TortoiseSVN-dev <[email protected]> Wed, 3 Jul 2024 09:56:08 -0700 (PDT)
Newsgroups gmane.comp.version-control.subversion.tortoisesvn.devel
Message-ID <[email protected]>
------=_Part_107135_1774590526.1720025768865
Content-Type: multipart/alternative; 
	boundary="----=_Part_107136_455975388.1720025768865"

------=_Part_107136_455975388.1720025768865
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable



On Wednesday, July 3, 2024 at 3:40:54=E2=80=AFPM UTC+2 [email protected]=
m wrote:

Hi,

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

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

[[[
TortoiseUDiff: Improve error reporting

Based on TortoiseGit rev. 0cadd137babbbd87ba834020b101d05d21b7e8e4.
]]]

The code (only relevant lines):
[[[
    BOOL bRet  =3D ReadFile(hFile, data, sizeof(data), &dwRead, nullptr);
    while ((dwRead > 0) && (bRet))
    {
... send data to Scintilla and check status ...
        bRet =3D ReadFile(hFile, data, sizeof(data), &dwRead, nullptr);
    }
    if (!bRet)
    {
        if (hFile || wantStdIn)
        {
            MessageBox(*this,=20
static_cast<LPCWSTR>(CFormatMessageWrapper()), L"TortoiseUDiff",=20
MB_ICONEXCLAMATION);
            return false;
        }
...
     }
]]]

What seems to happen is that ReadFile returns when there is no more data.=
=20
This causes MessageBox to be called with GetLastError() being=20
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=
=20
the check for if (hFile || wantStdIn). wantStdIn is set if TortoiseUDiff is=
=20
called with /b, but documentation says /b is optional and STDIN will be=20
used by default.


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


Maybe a check for GetLastError() =3D=3D ERROR_BROKEN_PIPE? I can't figure o=
ut=20
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=
=20
when the source process (e.g. git.exe when doing a 'git diff |=20
tortoiseudiff.exe') ends.

I'll commit a fix soon.

Stefan

--=20
You received this message because you are subscribed to the Google Groups "=
TortoiseSVN-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/=
tortoisesvn-dev/461ed213-083b-47d2-9a39-80695c2af5fbn%40googlegroups.com.

------=_Part_107136_455975388.1720025768865
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<br /><br /><div><div dir=3D"auto">On Wednesday, July 3, 2024 at 3:40:54=E2=
=80=AFPM UTC+2 [email protected] wrote:<br /></div><blockquote style=3D=
"margin: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204, 204); padd=
ing-left: 1ex;"><div>Hi,</div><div><br /></div><div>As noted in the Tortois=
eSVN 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 us=
es git but it can be reproduced with any source (for example piping from sv=
n).</div><div><br /></div><div>The root cause seems to be the changes by cs=
ware in r29665 on December 18th (2023):</div><div><br /></div><div>[[[</div=
><div>TortoiseUDiff: Improve error reporting<br /><br />Based on TortoiseGi=
t rev. 0cadd137babbbd87ba834020b101d05d21b7e8e4.<br />]]]<br /></div><div><=
br /></div><div>The code (only relevant lines):</div><div>[[[</div><div>=C2=
=A0 =C2=A0 BOOL bRet =C2=A0=3D ReadFile(hFile, data, sizeof(data), &amp;dwR=
ead, nullptr);<br />=C2=A0 =C2=A0 while ((dwRead &gt; 0) &amp;&amp; (bRet))=
<br />=C2=A0 =C2=A0 {</div><div>... send data to Scintilla and check status=
 ...<br />=C2=A0 =C2=A0 =C2=A0 =C2=A0 bRet =3D ReadFile(hFile, data, sizeof=
(data), &amp;dwRead, nullptr);<br />=C2=A0 =C2=A0 }<br />=C2=A0 =C2=A0 if (=
!bRet)<br />=C2=A0 =C2=A0 {<br />=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (hFile || w=
antStdIn)<br />=C2=A0 =C2=A0 =C2=A0 =C2=A0 {<br />=C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 MessageBox(*this, static_cast&lt;LPCWSTR&gt;(CFormatMessa=
geWrapper()), L"TortoiseUDiff", MB_ICONEXCLAMATION);<br />=C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 return false;<br />=C2=A0 =C2=A0 =C2=A0 =C2=A0 }<b=
r />...</div><div>=C2=A0 =C2=A0 =C2=A0}<br /></div><div>]]]<br /></div><div=
><br /></div><div>What seems to happen is that ReadFile returns when there =
is no more data. This causes MessageBox to be called with GetLastError() be=
ing ERROR_BROKEN_PIPE.</div><div><br /></div><div>I think the same would ha=
ppen with TortoiseGitUDiff (see [2]).</div><div><br /></div><div>I'm not su=
re I understand exactly what is supposed to happen, in particular the check=
 for if (hFile || wantStdIn). wantStdIn is set if TortoiseUDiff is called w=
ith /b, but documentation says /b is optional and STDIN will be used by def=
ault.</div></blockquote><div><br /></div><div>I think 'wantStdIn' is used t=
o show errors in case stdin is not available. Because in that case, hFile w=
ould be null and no error would be reported.</div><div>=C2=A0</div><blockqu=
ote style=3D"margin: 0px 0px 0px 0.8ex; border-left: 1px solid rgb(204, 204=
, 204); padding-left: 1ex;"><div><br /></div><div>Maybe a check for GetLast=
Error() =3D=3D ERROR_BROKEN_PIPE? I can't figure out from the documentation=
 for ReadFile [4] how to detect EOF for a pipe.</div></blockquote><div><br =
/></div><div>Yes, that's the correct solution. Because when piping, the pip=
e gets broken when the source process (e.g. git.exe when doing a 'git diff =
| tortoiseudiff.exe') ends.</div><div><br /></div><div>I'll commit a fix so=
on.</div><div><br /></div><div>Stefan</div><div><br /></div></div>

<p></p>

-- <br />
You received this message because you are subscribed to the Google Groups &=
quot;TortoiseSVN-dev&quot; group.<br />
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to <a href=3D"mailto:[email protected]">tor=
[email protected]</a>.<br />
To view this discussion on the web visit <a href=3D"https://groups.google.c=
om/d/msgid/tortoisesvn-dev/461ed213-083b-47d2-9a39-80695c2af5fbn%40googlegr=
oups.com?utm_medium=3Demail&utm_source=3Dfooter">https://groups.google.com/=
d/msgid/tortoisesvn-dev/461ed213-083b-47d2-9a39-80695c2af5fbn%40googlegroup=
s.com</a>.<br />

------=_Part_107136_455975388.1720025768865--

------=_Part_107135_1774590526.1720025768865--