Re: Review Request: Fix hang in kcm_useraccount

2012-08-15 Thread Oswald Buddenhagen
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/#review17428 --- heh - nasty: you put the dialog invocation into the backend cla

Re: Review Request: Fix hang in kcm_useraccount

2012-08-14 Thread Michael Palimaka
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/ --- (Updated Aug. 14, 2012, 12:33 p.m.) Review request for KDE Base Apps. Ch

Re: Review Request: Fix hang in kcm_useraccount

2012-08-11 Thread Michael Palimaka
On 2012-08-11 05:22, Rolf Eike Beer wrote And after the readLine() you have valid input? And you are sure your new branch is taken? Eike It seems it is the block parameter that makes all the difference: Test::Test(biol block) { QByteArray line; QList args; args += "201

Re: Review Request: Fix hang in kcm_useraccount

2012-08-11 Thread Oswald Buddenhagen
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/#review17253 --- now i took the time to have a deep look at this stuff ... the k

Re: Review Request: Fix hang in kcm_useraccount

2012-08-10 Thread Rolf Eike Beer
Am Samstag, 11. August 2012, 01:11:52 schrieb Michael Palimaka: > On 2012-08-11 01:02, Rolf Eike Beer wrote: > > Am Samstag 11 August 2012, 00:11:37 schrieb Michael Palimaka: > >> So you are saying: > >> > >> - readAll clears the buffer > >> - readLine unconditionally calls readAll > >> - readLine

Re: Review Request: Fix hang in kcm_useraccount

2012-08-10 Thread Michael Palimaka
On 2012-08-11 01:02, Rolf Eike Beer wrote: Am Samstag 11 August 2012, 00:11:37 schrieb Michael Palimaka: So you are saying: - readAll clears the buffer - readLine unconditionally calls readAll - readLine may be called multiple times to read multiple lines Is that correct? That's how the code

Re: Review Request: Fix hang in kcm_useraccount

2012-08-10 Thread Michael Palimaka
On 2012-08-10 02:46, Rolf Eike Beer wrote: Ehm, no. readLine() unconditionally calls readAll(). But that will also clear the input buffer of the process object. So there is nothing left to read on the next readLine() or readAll() call. Eike So you are saying: - readAll clears the buffer - rea

Re: Review Request: Fix hang in kcm_useraccount

2012-08-10 Thread Rolf Eike Beer
Am Samstag 11 August 2012, 00:11:37 schrieb Michael Palimaka: > On 2012-08-10 02:46, Rolf Eike Beer wrote: > > Ehm, no. readLine() unconditionally calls readAll(). But that will also > > clear the input buffer of the process object. So there is nothing left to > > read on the next readLine() or rea

Re: Review Request: Fix hang in kcm_useraccount

2012-08-09 Thread Rolf Eike Beer
Am Donnerstag, 9. August 2012, 20:24:26 schrieb Michael Palimaka: > On 2012-08-09 19:57, Rolf Eike Beer wrote: > > Ehm, no. If you don't unreadLine() but readLine() you will now discard > > the data previously read in by readAll() and only have the new input > > from readLine() which will be empty

Re: Review Request: Fix hang in kcm_useraccount

2012-08-09 Thread Michael Palimaka
On 2012-08-09 19:57, Rolf Eike Beer wrote: Ehm, no. If you don't unreadLine() but readLine() you will now discard the data previously read in by readAll() and only have the new input from readLine() which will be empty for sure. The case I'm describing here is: the process already exited and has

Re: Review Request: Fix hang in kcm_useraccount

2012-08-09 Thread Rolf Eike Beer
Am 2012-08-09 09:41, schrieb Michael Palimaka: On 2012-08-09 17:06, Rolf Eike Beer wrote: 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

Re: Review Request: Fix hang in kcm_useraccount

2012-08-09 Thread Michael Palimaka
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/ --- (Updated Aug. 9, 2012, 7:50 a.m.) Review request for KDE Base Apps. Chan

Re: Review Request: Fix hang in kcm_useraccount

2012-08-09 Thread Michael Palimaka
On 2012-08-09 17:06, Rolf Eike Beer wrote: You have a nice way trying to say I'm a moron. Thank you. ;) I am not trying to say that! I do not pretend to be any expert, and and the more eyes the better. Your patch isn't technically ideal as it does the same operation on data multiple times. B

Re: Review Request: Fix hang in kcm_useraccount

2012-08-09 Thread Michael Palimaka
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 origina

Re: Review Request: Fix hang in kcm_useraccount

2012-08-09 Thread Rolf Eike Beer
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 corre

Re: Review Request: Fix hang in kcm_useraccount

2012-08-08 Thread Rolf Eike Beer
Am Mittwoch 08 August 2012, 07:59:07 schrieb Michael Palimaka: > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105895/ > --- > > (U

Re: Review Request: Fix hang in kcm_useraccount

2012-08-08 Thread Michael Palimaka
> On Aug. 7, 2012, 7:26 a.m., Oswald Buddenhagen wrote: > > did you look at the changes i did in kdelibs/kdesu a while ago? this should > > probably go in line with them (i didn't check whether it already does). > > Michael Palimaka wrote: > What should I be looking for? > > Oswald Buddenh

Re: Review Request: Fix hang in kcm_useraccount

2012-08-08 Thread Michael Palimaka
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/ --- (Updated Aug. 8, 2012, 7:59 a.m.) Review request for KDE Base Apps. Chan

Re: Review Request: Fix hang in kcm_useraccount

2012-08-08 Thread Michael Palimaka
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/ --- (Updated Aug. 7, 2012, 6:52 p.m.) Review request for KDE Base Apps. Chan

Re: Review Request: Fix hang in kcm_useraccount

2012-08-07 Thread Oswald Buddenhagen
> On Aug. 7, 2012, 7:26 a.m., Oswald Buddenhagen wrote: > > did you look at the changes i did in kdelibs/kdesu a while ago? this should > > probably go in line with them (i didn't check whether it already does). > > Michael Palimaka wrote: > What should I be looking for? everything. it's n

Re: Review Request: Fix hang in kcm_useraccount

2012-08-07 Thread Rolf Eike Beer
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 s

Re: Review Request: Fix hang in kcm_useraccount

2012-08-07 Thread Rolf Eike Beer
> On Aug. 6, 2012, 4:55 p.m., Rolf Eike Beer wrote: > > kdepasswd/kcm/chfnprocess.cpp, line 68 > > > > > > Why unread the line if you read it again just 3 lines below? Why not > > just put the readline() below in an

Re: Review Request: Fix hang in kcm_useraccount

2012-08-07 Thread Michael Palimaka
> On Aug. 6, 2012, 4:55 p.m., Rolf Eike Beer wrote: > > kdepasswd/kcm/chfnprocess.cpp, line 68 > > > > > > Why unread the line if you read it again just 3 lines below? Why not > > just put the readline() below in an

Re: Review Request: Fix hang in kcm_useraccount

2012-08-07 Thread Michael Palimaka
> On Aug. 7, 2012, 7:26 a.m., Oswald Buddenhagen wrote: > > did you look at the changes i did in kdelibs/kdesu a while ago? this should > > probably go in line with them (i didn't check whether it already does). What should I be looking for? - Michael ---

Re: Review Request: Fix hang in kcm_useraccount

2012-08-07 Thread Oswald Buddenhagen
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/#review17018 --- did you look at the changes i did in kdelibs/kdesu a while ago?

Re: Review Request: Fix hang in kcm_useraccount

2012-08-06 Thread Michael Palimaka
> On Aug. 6, 2012, 6:09 p.m., Raphael Kubo da Costa wrote: > > How does this play with https://git.reviewboard.kde.org/r/104439 ? That review addresses a (different) case where chfn to produces a different output to what the kcm is expecting. Unfortunately I do not run either of those distros

Re: Review Request: Fix hang in kcm_useraccount

2012-08-06 Thread Michael Palimaka
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/ --- (Updated Aug. 6, 2012, 5:03 p.m.) Review request for KDE Base Apps. Chan

Re: Review Request: Fix hang in kcm_useraccount

2012-08-06 Thread Raphael Kubo da Costa
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/#review16989 --- How does this play with https://git.reviewboard.kde.org/r/10443

Re: Review Request: Fix hang in kcm_useraccount

2012-08-06 Thread Rolf Eike Beer
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/#review16985 --- kdepasswd/kcm/chfnprocess.cpp

Review Request: Fix hang in kcm_useraccount

2012-08-06 Thread Michael Palimaka
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105895/ --- Review request for KDE Base Apps. Description --- When changing the u