> On Oct. 2, 2015, 10:35 p.m., David Faure wrote:
> > Sending Activate to a running Unique process is not a failure, so
> > NoExitOnFailure should not affect it.
> >
> > If you need a "no exit, ever" flag, then I would recommend adding a
> > separate flag for that. I'm not sure I understand the use case though.
>
> Martin Klapetek wrote:
> In terms of the this code, the "failure" is "!d->registered" (see line
> 147 or 153 with this patch), so it is a failure to register the service (this
> is also supported by the docs saying "Indicates that the application should
> not exit if it failed to register with D-Bus."). So even if it sends
> Activate, it still failed to register the service and therefore the
> NoExitOnFailure should apply. No?
>
> David Faure wrote:
> While that might be one way to literally interpret the docs, it's
> definitely not what I had in mind when I wrote them.
> See "Already running so it's ok!", the fact that there is no
> "errorMessage" set in this case, and the fact that the current use of
> NoExitOnFailure happens right before qCritical() + exit.
>
> Process already running is not an error or a failure, it's a perfectly
> normal situation.
>
> What I can't really remember is the use case for NoExitOnFailure; I think
> it was "being able to start the app even if there's no DBus running at all",
> at the expense of losing the Unique behavior of course.
>
> Back to your use case, I wonder why "pass data" can't be done with the
> CommandLine method. Or are we talking about different data than the command
> line? Which other data could there be, in an application we just started ?
>
> Martin Klapetek wrote:
> > Process already running is not an error or a failure, it's a perfectly
> normal situation.
>
> It isn't, the "failure" in this context of KDBusService as I understand
> it from the docs and the code is just the failure to register the unique dbus
> service, which I thought the NoExitOnFailure is meant for.
>
> I mean it would be nice if the user had a choice to not forcefully exit
> when it cannot get the unique dbus service.
>
> Anyways, the main reason for this patch comes from kwalletd, which has
> this code:
>
> KDBusService dbusUniqueInstance(KDBusService::Unique |
> KDBusService::NoExitOnFailure);
> // NOTE: the command should be parsed only after KDBusService
> instantiation
> QCommandLineParser cmdParser;
> aboutdata.setupCommandLine(&cmdParser);
> cmdParser.process(app);
>
>
> if (!dbusUniqueInstance.isRegistered()) {
> qDebug() << "kwalletd is already running!";
> return 1;
> }
>
> Now the code exits at the first line and never reaches the if below,
> which I originally wanted to extend to instead check for a locked wallet and
> unlock it with PAM (in case this kwalletd was executed from PAM) and only
> then actually exit.
>
> Martin Klapetek wrote:
> The other usecase for this patch can be seen in the code excerpt above -
> returning custom values in main(). Eg. kwalletd would return 1 in case it
> couldn't register on dbus, but KDBusService exits with 0 instead.
>
> Martin Klapetek wrote:
> (I just reread my reply above and the first "It isn't," was meant as a
> reply to the "...is not an error or a failure", not to "it's a perfectly
> normal situation". #context)
>
> David Faure wrote:
> I think I know better than you what I meant when I wrote the code and the
> docu :-) I agree that the docu might be unclear, but don't tell me I meant
> something else :-)
> By failure I meant an error (like no dbus session found).
> "Unique app already running" is *not* a failure. Look at the kdelib4s
> times: kmail & ; kmail & -> return code 0.
>
> I don't see a strong reason for kwalletd to return 1 instead of 0 if it's
> already running. The user wanted kwalletd to run, it happens to run already,
> there's no error there.
>
> For your use case of unlocking with PAM, it seems to me that there's an
> alternative (possibly better) solution than a NoExit flag anyway. Before all
> the code above,
> 1) use KWallet API to check if the wallet is opened (if kwalletd isn't
> running, it's not; but maybe kwalletd is running and the wallet is closed).
> (so using KDBusService to check if it's running only does half the job
> anyway, so better do what you need to do right away)
> 2) if the wallet is closed, open it with PAM
> 3) and *then* create the KDBusService.
>
> Does that make sense? [I don't know much about PAM; and in all cases I'm
> not sure how an already running kwalletd suddenly sees the wallet being open]
>
> It just seems to me that you're trying to abuse "server-side" API
> (KDBusService) for a client-side check (is the daemon already running?).
> It just seems to me that you're trying to abuse "server-side" API
> (KDBusService) for a client-side check (is the daemon already running?).
Hmm, perhaps. Regardless of kwallet, I still think it'd make sense to give more
control to the user of KDBusService rather than force-exit. But that's fine,
I'll see what alternatives we can do for kwallet.
- Martin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125483/#review86249
-----------------------------------------------------------
On Oct. 2, 2015, 7:52 p.m., Martin Klapetek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125483/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2015, 7:52 p.m.)
>
>
> Review request for KDE Frameworks and David Faure.
>
>
> Repository: kdbusaddons
>
>
> Description
> -------
>
> If KDBusService fails to register Unique service because other instance is
> already running and if the user specified NoExitOnFailure, don't exit after
> calling "Activate" on the other instance.
>
>
> Diffs
> -----
>
> src/kdbusservice.cpp ea7727d
>
> Diff: https://git.reviewboard.kde.org/r/125483/diff/
>
>
> Testing
> -------
>
> App no longer exits and can do some other tasks like calling a different dbus
> method to eg. pass data and then exit on its own.
>
>
> Thanks,
>
> Martin Klapetek
>
>
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
