<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/108385/">http://git.reviewboard.kde.org/r/108385/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 13th, 2013, 6:45 p.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">and why exactly do you sleep instead of looping with waitforreadyread?</pre>
</blockquote>
<p>On January 13th, 2013, 7:08 p.m. UTC, <b>Jon Severinsson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Because that would be an (almost) busy-loop (there are already *some* data, so waitForReadyRead could return before the timeout).</pre>
</blockquote>
<p>On January 13th, 2013, 7:35 p.m. UTC, <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">right. my (kpty) implementation waits for *any* data to be available (which is (or was) consistent with something i don't remember), while thiago's (qsocket & co.) implementations wait for *more* data to be available. this really should be made consistent at some point ... at this point i think thiago's interpretation is more useful, even if it means writing more code in the common case.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have not looked too closely at either implementation, and can't say I really understand how either actually work, so this might be pointless, but I would still want to give a warning about the whole "waiting for *more* data" (as opposed to "waiting for *some* data") concept, as that immediately brings to question "more since *when*?".
More data since the waitForReadyRead() call *can* *not* be used correctly. Never under any circumstance! Every call would be a race condition (all data that is ever going to come might have come just before you made the call), and would at best result in an unnecessary sleep of "timeout" ms, and at worst result in a complete deadlock, or if the rest of the system conspires against you, an infinite loop.
More data since last "relevant" API call prior to waitForReadyRead() could work, but defining "relevant", and getting both the implementation and documentation right would likely be a nightmare, and chances a API consumer makes a mistake is still possible, even likely, so this is not something I would recommend.
So imho "waiting for *some* data" is the only right thing to do, even though it results in this ugly code in some cases...</pre>
<br />
<p>- Jon</p>
<br />
<p>On January 13th, 2013, 1:03 p.m. UTC, Jon Severinsson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Jon Severinsson.</div>
<p style="color: grey;"><i>Updated Jan. 13, 2013, 1:03 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Increase timeout, and sleep a while after waitForReadyRead() returns,
as it only guarantees that *some* data is available to read, while
the test assumes that a full line of data is available to read...
This reduces failure rate from 10% to 2% on my setup.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kpty/tests/kptyprocesstest.cpp <span style="color: grey">(b95ae26)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108385/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>