> On Dec. 24, 2011, 8:28 a.m., David Faure wrote:
> > This seems like a dangerous change to me.
> > 
> > With it, you could run multiple instance of "kmail --nofork", and they 
> > would step on each other's toes. The whole point of kuniqueapplication (in 
> > normal apps, not in konsole), is to prevent multiple instances of the 
> > application from running.
> > A developer using --nofork in valgrind or gdb shouldn't run the risk that 
> > an app that wasn't mean to run twice, ends up running twice.
> > 
> > konsole is doing rather unusual stuff with kuniqueapplication so I can't 
> > comment on what would be the proper fix for konsole, but I'm pretty sure it 
> > should only affect konsole, not other kuniqueapplications.
> 
> Thomas Lübking wrote:
>     Seconded - "Do NOT ship it!"
>     
>     $ <random kuniqueapplication> --help-kde
>     ...
>       --nofork                  Do not run in the background.
>     
>     
>     There's no indication this means to spawn a new process and it's (despite 
> the ambigious term) obviously not meant like this (but meaning "do not fork 
> to background")
>     
>     -> Solution:
>     Konsole should add an extra parameter or map it's --nofork to 
> NonUniqueInstance in the ::start() call.
>     
>     Turning "--nofork" to "--new-process" for general kuniqueapplication is 
> rather a no-go, because applications like eg. k3b or amarok could possibly 
> really rely on it (ioctl/database messup)
>     
>     I do see your point in either having an extra konsole process as well as 
> in redirecting IO, but please fix it in konsole *only*
> 
> David Faure wrote:
>     Don't get confused by the variable name. AFAICS "forceNewProcess" only 
> means that it will register to DBus as kfoo-PID instead of kfoo.
>     Nofork doesn't fork, the patch doesn't change that, AFAICS.
>

Looking at the patch, setting that flag would get him into the block he desires 
so whatever the effect of the patch is, setting the flag should gain the same, 
yesno?!


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103333/#review9226
-----------------------------------------------------------


On Dec. 23, 2011, 11:58 a.m., Askar Safin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103333/
> -----------------------------------------------------------
> 
> (Updated Dec. 23, 2011, 11:58 a.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> See https://bugs.kde.org/show_bug.cgi?id=288200
> 
> 
> This addresses bug 288200.
>     http://bugs.kde.org/show_bug.cgi?id=288200
> 
> 
> Diffs
> -----
> 
>   kdeui/kernel/kuniqueapplication.cpp 777fc35 
> 
> Diff: http://git.reviewboard.kde.org/r/103333/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Askar Safin
> 
>

Reply via email to