<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/127108/">https://git.reviewboard.kde.org/r/127108/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On März 1st, 2016, 8:16 nachm. CET, <b>Christian David</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/127108/diff/2/?file=447715#file447715line1185" style="color: black; font-weight: bold; text-decoration: underline;">kmymoney/views/kmymoneyview.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1176</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_oldMask</span><span class="p">{</span><span class="n">umask</span><span class="p">((</span><span class="o">~</span><span class="n">mode</span><span class="p">)</span> <span class="o">&</span> <span class="k">static_cast</span><span class="o"><</span><span class="kt">mode_t</span><span class="o">></span><span class="p">(</span><span class="mo">0777</span><span class="p">))}</span></pre></td>
</tr>
</tbody>
</table>
<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;">In this line I get a warning which I do not understand:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">kmymoney/views/kmymoneyview.cpp: In constructor ‘KMyMoneyView::saveToLocalFile(const QString&, IMyMoneyStorageFormat*, bool, const QString&)::restorePreviousSettingsHelper::restorePreviousSettingsHelper(mode_t)’:
kmymoney/views/kmymoneyview.cpp:1176:22: warning: narrowing conversion of ‘umask(((~ mode) & 511u))’ from ‘__mode_t {aka unsigned int}’ to ‘int’ inside { } [-Wnarrowing]
m_oldMask{umask((~mode) & static_cast<mode_t>(0777))}
</pre></div>
</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">You convert an 'unsigned int' (the type of the result of your expression) to an 'int' (the type of m_oldMask) and loose one bit of information (since the int needs one bit over the unsigned for its sign bit). This does not hurt here, as only 9 bits are relevant.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You can try if</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">m_oldMask { static_cast<int>((~mode) & 0777u) }</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">or make m_oldMask of type mode_t or variations thereof.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On März 1st, 2016, 8:16 nachm. CET, <b>Christian David</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/127108/diff/2/?file=447716#file447716line162" style="color: black; font-weight: bold; text-decoration: underline;">libkgpgfile/kgpgfile.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">162</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// qDebug("check access rights");</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<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;">I removed this test because <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QFile::open()</code> will check that anyway and more important: This can be wrong under some circumstances.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">Not sure. Please check against https://bugs.kde.org/show_bug.cgi?id=256750 for which this has been added.</p></pre>
<br />
<p>- Thomas</p>
<br />
<p>On März 1st, 2016, 8:03 nachm. CET, Christian David 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 KMymoney.</div>
<div>By Christian David.</div>
<p style="color: grey;"><i>Updated März 1, 2016, 8:03 nachm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=356399">356399</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kmymoney
</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;">Fixed potential memory leak during saving</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">A pointer was not deleted before throwing exceptions in
KMyMoneyView::saveFile. Also renamed the pointer.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Changed way of saving files which should fix some bugs</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The save operation seems to fail every time - it never changed the
orginal file and never reported any issues. I did not find the exact
reason for this bug but I am quite sure it was caused by an incorret
usage of QSaveFile (under some circumstances close() instead of commit()
was called).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Now KMyMoney creates its own temporary file to write to (if needed).
This also works using KGpgFile, which should fix Bug 356399.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The remove() and rename() operations are not atomic which is not so
good as this could result in dataloss if the first operation fails.
However, this is the best OS independet process I could find.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Errors during writing of compressed files may not be detected. I think
this issue should be fixed upstream.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">BUG: 356399
FIXED-IN: 5.0
REVIEW: 127108</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Fixed bug while saving GPG encrypted files</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">A call to QSaveFile::commit() was missing. So QSaveFile always assumed
an error and discarded the changes.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Used this fix for minor clean-ups.</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;">I saved a file several times using the compressed, uncompressed and anonymous format. I could not test the GPG part because none of my keys is currently shown by the save dialog.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New: I tested it with GPG encrypted files.</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>CMakeLists.txt <span style="color: grey">(a82d70eed87c5f8a8052b969c0a8a17d82ef8b1d)</span></li>
<li>kmymoney/views/kmymoneyview.h <span style="color: grey">(c4a769c2bf88083ea56283d683d7f0f7f0466875)</span></li>
<li>kmymoney/views/kmymoneyview.cpp <span style="color: grey">(284bd6a2657982c25790b2428730f279fc86504c)</span></li>
<li>libkgpgfile/kgpgfile.cpp <span style="color: grey">(b1870be92edb833ed30f369e3e0ca0f320fe147b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127108/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>