D10641: Revoke temporary authorization of KIO slave before sending status to IdleSlave

[David Faure]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180304/5b8fc952/attachment-0001.html>