> 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.
>
> 
> Thomas Lübking wrote:
>     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?!
> 
> Askar Safin wrote:
>     Yes, you are right. So, please add to konsole/src/main.cpp to function 
> forceNewProcess() check, wherever or not there is option "--nofork".
> 
> Askar Safin wrote:
>     Thomas, if you apply the patch, konsole will start in new process if 
> there is --nofork option independently of terminal status. But, as I said, my 
> patch is bad idea, edit konsole/src/main.cpp
> 
> Thomas Lübking wrote:
>     I didn't mean to question that result, just that that's not the purpose 
> of --nofork in general. Konsole somehow abuses it ;-)
>     
>     PS: i'm not the maintainer of konsole, so i cannot just add that check 
> there across whoever /is/ the maintainer, sorry.
> 
> Askar Safin wrote:
>     What can I do to apply change to konsole?
>     I cannot post it to reviewboard, because I cannot write patch (trivial 
> solution 'return (isatty(1) && !args->isSet("new-tab")) || 
> args->isSet("no-fork")' suddenly doesn't work, because option "no-fork" is in 
> another container).
>     I cannot post it to bugs.kde.org, because there bugs stay opened very 
> long time.
>     How to contact with konsole maintainers?
> 
> David Faure wrote:
>     Use
>        !args->isSet("fork");
>     
>     The option is named fork and default to true, --no-fork is the way to set 
> it to false.
>     
>     According to git log the last one from `konsole --author` who committed 
> to the code was Kurt Hindenburg <kurt.hindenb...@gmail.com>. Just a month 
> ago, so he should still be around :-)

Thanks, David. I send e-mail to Kurt. !args->isSet("fork") doesn't work, too. I 
tried it. Option "--fork" or "--no-fork" is (as I said) in another container. 
Not in "KCmdLineArgs::parsedArgs()", but in "kuniqueapplication::parsedArgs()" 
or something like that. But if I write the following:

return (isatty(1) || !kuniqueapplication::parsedArgs()->isSet("fork")) && 
!KCmdLineArgs::parsedArgs()->isSet("new-tab");

program crashes with fatal error or segmentation fault


- Askar


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


On Dec. 25, 2011, 9:01 a.m., Askar Safin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103333/
> -----------------------------------------------------------
> 
> (Updated Dec. 25, 2011, 9:01 a.m.)
> 
> 
> Review request for kdelibs and Konsole.
> 
> 
> 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