D10822: Store temporary authorization status in IdleSlave

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> idleslave.cpp:82
>          // Overload with (bool) onHold, (QUrl) url.
>          if (!stream.atEnd()) {
>              QUrl url;

Reading tempAuth before url breaks protocol, i.e. people who upgrade KF5 in a running plasma session will be in big trouble.

At least check if (!stream.atEnd()) before stream >> tempAuth.

Hmm unfortunately that "last optional argument" URL breaks any possibility to make the change fully compatible :(

How about this? Define MSG_SLAVE_STATUS_V2 which always sends all its arguments (including tempAuth, onHold, and url), and modify SlaveBase to send that, and support both here, at least temporarily?
New KIO on old klauncher will lead to "Unexpected data from KIO slave" (unknown command, just above) but that'll be a clear message to logout or restart kdeinit5, while deserializing a bool into a QUrl or vice-versa is just messy and could lead to very strange and hard to debug behaviour.

And this way in the future we don't need a V3, we can just append new arguments, with a atEnd() check.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10822

To: chinmoyr, dfaure
Cc: #frameworks, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180301/b5cb8866/attachment.html>