Review Request: Fix hang in kcm_useraccount

Rolf Eike Beer kde at opensource.sf-tec.de
Thu Aug 9 08:06:39 BST 2012


Michael Palimaka wrote:
> On 2012-08-09 02:59, Rolf Eike Beer wrote:
> > Now we have the following situation in case the program has exited (if not
> > the behavior is unchanged):
> > 
> > -we read in one line, if it is empty we break. What happens if the first
> > line of output is empty and correct output comes later?
> 
> That's why I originally went for readAll.

You could have just said that instead of taking my suggestions ;)

> > -the line is not empty, we continue (new check)
> > 
> > -we check if the line is empty (old check)
> > 
> > So, what now?
> > 
> > I would suggest the following:
> > 
> > -readAll()
> > -if empty, break
> > -find a linebreak, if we have one: unreadLine for everything beyond it,
> > cut
> > the input line we have
> > -move the readLine() and the isEmpty() of the old code together in the
> > else
> > (i.e. pidExited == NotExited).

> Isn't that what effectively what the original revision does (dropping
> the unread since readLine after readAll is safe)?
> 
> If the program has exited:
> - Read all output
> - If no output is found, break
> - If output is found, continue along the original code
> 
> The purpose of this change is only to break if the program has exited
> without output.

You have a nice way trying to say I'm a moron. Thank you. ;)

Your patch isn't technically ideal as it does the same operation on data 
multiple times. But seriously: we have wasted way more time and computing 
power by doing this mail thread then the most ideal code solution will ever be 
able to return.

Just one thing: you do unreadLine(line), which adds "true" as second argument, 
so a needless newline is appended that wasn't there before (and which causes a 
deep copy of all that stuff for nothing). So unreadLine(line, false) should it 
be. And I will not speak up again on this unless explicitely requested to ;)

Eike
-------------- 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/20120809/b72fba86/attachment.sig>


More information about the kde-core-devel mailing list