Review Request 130117: Fix detection of EOF

[Peter Wu]

> On May 8, 2017, 9:40 a.m., Oswald Buddenhagen wrote:
> > this looks familiar ... https://phabricator.kde.org/D4975
> > 
> > so assuming the condition is valid, this requires an _extensive_ comment.
> > also, shouldn't this just be unified with the solaris code path?
> 
> Martin Tobias Holmedahl Sandsmark wrote:
>     No, Solaris has a very different behavior that is described by the already existing comment (needing to "drain" the empty bytes with read(..., 0); etc.). It won't work on Linux.
>     
>     What stuff do you want in the comment? I described how detecting an EOF is supposed to be done (by actually trying to read).
> 
> Oswald Buddenhagen wrote:
>     the part i find interesting is the explanation of the confirmed legit situations where this can occur. judging by the phab diff, there is a change of behavior in the kernel involved. otoh, the bugzilla entry suggests that there is a behavior change in kpty, which doesn't ring a bell in to me. i want that cleared up, because it _smells_.

I agree with Oswald, while the fix might hide the issue, the real question is why it happens.

My patch tried to explain why it possibly happened, but it was still guesswork and not fully confirmed. This patch does not even explain why the assumption should be removed.

If you have time, please try to reach the kernel developers (ideally with a minimal testcase, one which I failed to create), maybe they have an idea.


- Peter


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130117/#review103203
-----------------------------------------------------------


On May 6, 2017, 8:09 p.m., Martin Tobias Holmedahl Sandsmark wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130117/
> -----------------------------------------------------------
> 
> (Updated May 6, 2017, 8:09 p.m.)
> 
> 
> Review request for Konsole, Kurt Hindenburg, Oswald Buddenhagen, and Peter Wu.
> 
> 
> Bugs: 372991
>     https://bugs.kde.org/show_bug.cgi?id=372991
> 
> 
> Repository: kpty
> 
> 
> Description
> -------
> 
> Don't just assume that 0 bytes read means EOF.
> 
> 
> Diffs
> -----
> 
>   src/kptydevice.cpp 22233a5 
> 
> Diff: https://git.reviewboard.kde.org/r/130117/diff/
> 
> 
> Testing
> -------
> 
> Fixes the konsole issue from https://phabricator.kde.org/D4975 and https://bugs.kde.org/show_bug.cgi?id=372991 and the autotest works.
> 
> 
> Thanks,
> 
> Martin Tobias Holmedahl Sandsmark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20170508/3ae0c9b9/attachment.html>