dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> slavebase.cpp:147
>
> - bool hasTempAuth()
> + bool hasTempAuth(bool revoke = false)
> {
That looks like horrible API ;)
A getter that optionally makes changes, based on a boolean... urgh ;)
"Duplicating" a for loop isn't really duplication, go for a different method.
> slavebase.cpp:554
> qint8 b = connected ? 1 : 0;
> - KIO_DATA << pid << mProtocol << host << b << d->hasTempAuth();
> + KIO_DATA << pid << mProtocol << host << b << d->hasTempAuth(true);
> if (d->onHold) {
It turns out that indeed this method is only called when the slave is going to
Idle, but, hmm, in this code nothing says so, it looks like a method that just
sends current status, and that could be called at any moment...
I would suggest to at least add a comment here, something like
// This method is only called from the IdleSlave constructor, the slave has
just been returned to klauncher, clear any authorization so that another
application cannot benefit from it.
... above the method call that you'll have to add to clear the auths, due to
the previous comment ;)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10641
To: chinmoyr, dfaure
Cc: fvogt, #frameworks, michaelh
