<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119540/">https://git.reviewboard.kde.org/r/119540/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Juli 30th, 2014, 2:29 nachm. UTC, <b>Luca Beltrame</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">My question is, is KAuth being constructed with a default string ("") useful to anyone? IMO the issue, if any, it's there, and this looks like a workaround. (But I claim no expertise on the code, so feel free to flame me :P)</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">From what I understand: since kauth supports multiple backends assuming that an empty string necessarily represents an invalid action may well be wrong, the apidox at the very least do not say anything about this. It does however constitute an invalid action for polkit-qt, so there is a problem in polkit's authority class in that it either should reject operations on an empty action id preventing obvious errors or (somewhat more importantly I guess) recover from having used an invalid action id previously. At a polkit-qt level what happens is that the action results in a (IIRC) instance error which will make all further requests be discarded on account of the Authority having the error flag set, from that point on it does not automatically recover from. So to fix this particular polkit-qt specific issue I can actually imagine two ways this should work a) kauth should use clearError() before trying to do anything b) polkit-qt clearing the error when a new action(id) is requested. I totally do not feel comfortable saying which one it should be but from a convenience POV latter certainly seems best, having to clear error flags from outside the library is not a case I feel is often present in Qt-based libraries.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Regardless of all that, Auth(0) with 0 being implicitly cast to QString causing the Auth(QString&) ctor to be used certainly is not the intended initialization in kcmodule. So it happens to be a workaround for the Authority encountering an error and then refusing to do things until someone clears the error, it also happens to be a fix for using a wrong ctor and weirdly implict casting QString though ;)</p></pre>
<br />
<p>- Harald</p>
<br />
<p>On Juli 30th, 2014, 12:32 vorm. UTC, Harald Sitter wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks, Hrvoje Senjan and Martin Bříza.</div>
<div>By Harald Sitter.</div>
<p style="color: grey;"><i>Updated Juli 30, 2014, 12:32 vorm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kconfigwidgets
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KAuthAction(0) actually calls the QString constructor which will dispatch<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
bogus polkit auth checks, polkitqt and polkitd are not terribly happy about<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
those and subsequently don't want to talk to the KCM anymore, even if it<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
manages to create a proper Action</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">creating a null Action rather than accidentally triggering the QString<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
ctor ensures that we only do polkit interaction with reasonable actionids<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
making sure that authentication works as expected</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">tested with clock kcm, succesfully can talk with the helper app if the bogus actionid "" wasn't used intermediately</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/kcmodule.cpp <span style="color: grey">(92e5427c121491e4ebf289addda040cc117cdd68)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119540/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>