<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/120388/">https://git.reviewboard.kde.org/r/120388/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 29th, 2014, 3:58 a.m. IST, <b>David Faure</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;">low-level review: no need to create a QFile, use QFile::exists(path)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">higher-level review: ensuring that a file exists before calling KSaveFile, which <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">creates</em> the file, sounds weird. I mean, isn't this exactly what will happen the very first time a wallet is created? (disclaimer - I don't know this code so I'm probably missing context).</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">low-level review: no need to create a QFile, use QFile::exists(path)</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ok</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">higher-level review: ensuring that a file exists before calling KSaveFile, which creates the file, sounds weird. I mean, isn't this exactly what will happen the very first time a wallet is created? (disclaimer - I don't know this code so I'm probably missing context).</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If we delete the kwallet directory we would loose both the kwl as well as salt files. Now if we were to call sync, kwl file gets written to disk and user can continue to add data to the wallet untill he closes it, after which he no longer would be able to access it since he doesn't have the salt file with him. Any data that was added to the wallet in the meantime would be lost forever.</p></pre>
<br />
<p>- Arjun</p>
<br />
<p>On December 11th, 2014, 7:46 a.m. IST, Arjun AK 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, Àlex Fiestas, Teo Mrnjavac, and Valentin Rusu.</div>
<div>By Arjun AK.</div>
<p style="color: grey;"><i>Updated Dec. 11, 2014, 7:46 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwallet
</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;"><ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Create a new wallet "foo"</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">immediately remove the 'kwalletd5' directory</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"><code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">KWalletD::timedOutSync()</code> will call sync on a nonexistant wallet (which will create "foo.kwl")</li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Kwalletmanager still lists the wallet and new data can be added to the wallet, but this data will be lost once the wallet is closed since the salt has been deleted and therefore the wallet cannot be opened again.</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/runtime/kwalletd/backend/kwalletbackend.cc <span style="color: grey">(b072cec)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120388/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>