<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/101275/">http://git.reviewboard.kde.org/r/101275/</a>
</td>
</tr>
</table>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101275/diff/3/?file=16108#file16108line47" style="color: black; font-weight: bold; text-decoration: underline;">libs/internals/setting.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<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">47</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">virtual</span> <span class="kt">void</span> <span class="n">save</span><span class="p">(</span><span class="kt">int</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">change this to 'save (int scope)' and write a comment saying the scope value should be one of the Knm::Connection::Scope enumerate values, just to warn people to do not pass bogus values when using this function. After that and the other small changes in this review you can commit. Just remember to fix the potential memory leak I listed in a comment below.</pre>
</div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101275/diff/3/?file=16111#file16111line70" style="color: black; font-weight: bold; text-decoration: underline;">libs/internals/settings/802-1x.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString Security8021xSetting::importCertFromPath(const QString & oldpath, const QString & newpath, Knm::Connection::Scope scope)</pre></td>
</tr>
</tbody>
<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">70</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">code style for if</pre>
</div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101275/diff/3/?file=16120#file16120line91" style="color: black; font-weight: bold; text-decoration: underline;">libs/ui/security/tlswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void TlsWidget::readConfig()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">79</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">value</span> <span class="o">=</span> <span class="n">d</span><span class="o">-></span><span class="n">setting</span><span class="o">-></span><span class="n">clientcertpath</span><span class="p">();</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">87</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">else</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">more code style for if</pre>
</div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101275/diff/3/?file=16120#file16120line99" style="color: black; font-weight: bold; text-decoration: underline;">libs/ui/security/tlswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void TlsWidget::readConfig()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">86</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span> <span class="k">else</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">93</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">code style for if</pre>
</div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101275/diff/3/?file=16120#file16120line119" style="color: black; font-weight: bold; text-decoration: underline;">libs/ui/security/tlswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void TlsWidget::readConfig()</pre></td>
</tr>
</tbody>
<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">111</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">code style for if</pre>
</div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101275/diff/3/?file=16124#file16124line650" style="color: black; font-weight: bold; text-decoration: underline;">settings/config/manageconnectionwidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void ManageConnectionWidget::addGotConnection(bool valid, const QString &errorMessage)</pre></td>
</tr>
</tbody>
<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">647</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Knm</span><span class="o">::</span><span class="n">Connection</span> <span class="o">*</span><span class="n">con</span> <span class="o">=</span> <span class="k">new</span> <span class="n">Knm</span><span class="o">::</span><span class="n">Connection</span><span class="p">(</span><span class="n">QUuid</span><span class="p">(</span><span class="n">id</span><span class="p">),</span> <span class="n">type</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This connection object is temporary, right? So I think you should delete it along with the ConnectionPersistence object to avoid memory leak or you could use 'Knm::Connection con(...)' instead of dynamic allocating it.</pre>
</div>
<br />
<p>- Lamarque Vieira</p>
<br />
<p>On May 4th, 2011, 10:36 p.m., Ilia Kats wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Network Management.</div>
<div>By Ilia Kats.</div>
<p style="color: grey;"><i>Updated May 4, 2011, 10:36 p.m.</i></p>
<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;">Currently, KDE NM is not compatible with PEM certificates (base64-encoded), which are the most used format. Attached patch makes some improvements in certificate handling, namely:
until now, the certificate was stored in the connection settings file, not base64-decoded or anything, and passed like that to NM. However, NM expects a binary DER certificate if using the blob scheme or a path to the certificate in the form of file://path_to_certificate if using the path scheme. Since a PEM file containing multiple certificates is possible, we would have to use the path scheme anyway, as a DER blob can only contain one certificate, so this patch also improves the certificate handling: Instead of just saving the path to the certificate, it is now being imported to $HOME/.kde/share/networkmanagement/certificates for user scope connections or /usr/share/kde4/apps/networkmanagement/certificates for systemscope connections. This allows the user to move or delete the original file without worrying about his connections. Certificates are deleted when they are no longer needed, e.g. the connection gets deleted, "Use system CA" is checked or another security method is selected.
This is a major change in behavior, so I would like a public review.</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;">Yes, see the bugzilla ticket.</pre>
</td>
</tr>
</table>
<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=209673">209673</a>
</div>
<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">(6748cee)</span></li>
<li>backends/NetworkManager/nmdbussettingsconnectionprovider.cpp <span style="color: grey">(40a364f)</span></li>
<li>libs/internals/connection.h <span style="color: grey">(60516dc)</span></li>
<li>libs/internals/connection.cpp <span style="color: grey">(e00c8ac)</span></li>
<li>libs/internals/connectionpersistence.cpp <span style="color: grey">(2862250)</span></li>
<li>libs/internals/setting.h <span style="color: grey">(b43365b)</span></li>
<li>libs/internals/setting.cpp <span style="color: grey">(e66c65a)</span></li>
<li>libs/internals/settings/802-1x.h <span style="color: grey">(971ef41)</span></li>
<li>libs/internals/settings/802-1x.cpp <span style="color: grey">(245507e)</span></li>
<li>libs/internals/settings/802-1xpersistence.cpp <span style="color: grey">(b7b9c42)</span></li>
<li>libs/ui/security/eapmethodleap.cpp <span style="color: grey">(199a8b8)</span></li>
<li>libs/ui/security/eapmethodpeapbase.ui <span style="color: grey">(4b9c5fd)</span></li>
<li>libs/ui/security/eapmethodtlsbase.ui <span style="color: grey">(80a5c1f)</span></li>
<li>libs/ui/security/eapmethodttlsbase.ui <span style="color: grey">(4f2e1a9)</span></li>
<li>libs/ui/security/peapwidget.h <span style="color: grey">(51ad781)</span></li>
<li>libs/ui/security/peapwidget.cpp <span style="color: grey">(2fc502b)</span></li>
<li>libs/ui/security/tlswidget.h <span style="color: grey">(c71dcf3)</span></li>
<li>libs/ui/security/tlswidget.cpp <span style="color: grey">(1d28636)</span></li>
<li>libs/ui/security/ttlswidget.h <span style="color: grey">(b1a9049)</span></li>
<li>libs/ui/security/ttlswidget.cpp <span style="color: grey">(c135b19)</span></li>
<li>settings/config/manageconnectionwidget.h <span style="color: grey">(58afc12)</span></li>
<li>settings/config/manageconnectionwidget.cpp <span style="color: grey">(3c0d4dc)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101275/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>