Review Request: Fix hang in kcm_useraccount

Rolf Eike Beer kde at opensource.sf-tec.de
Tue Aug 7 20:51:32 BST 2012


Michael Palimaka wrote:

Switching to mail as reviewboard is once again playing tricks at me or my 
Konqueror.

> Changes
> -------
> 
> Restore original diff, and add comments explaining it.

That comment would explain what you are doing, but the idea was to explain 
_why_ you are doing it.

But since I started all this back and forth I think you deserve a technically 
correct review, so I looked into PtyProcess what is going on there. So I think 
that the second version is indeed the correct one (if at all). Rationale: 
readLine() just calls readAll() first, so there can't be anything wrong in 
calling readLine() after readAll().

That call to readAll() could also return more than just one line, so reading 
something out of the input buffer that should only be looked at in the next 
loop iteration or whatever. So from looking at the code of PtyProcess I think 
the correct thing would be to call readLine(false) instead of readAll(false). 
It will just behave the same if no complete line is available, but it will at 
most return one line. And then it is just fine to go ahead with whatever you 
got: it's either a complete line or there will be no more from the child as it 
is already gone.

So: version 2, with readAll(false) in line 60 replaced by readLine(false)

Other opinions anyone?

Eike

P.S.: thanks for caring for this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20120807/d5af403c/attachment.sig>


More information about the kde-core-devel mailing list