<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/111636/">http://git.reviewboard.kde.org/r/111636/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 23rd, 2013, 11:19 a.m. UTC, <b>David Faure</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="http://git.reviewboard.kde.org/r/111636/diff/1/?file=172718#file172718line523" style="color: black; font-weight: bold; text-decoration: underline;">staging/kio/src/core/authinfo.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">bool NetRC::lookup( const QUrl& url, AutoLogin& login, bool userealnetrc,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">469</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c"> AutoLogin l;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">441</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">d</span><span class="o">-></span><span class="n">fstream</span><span class="p">.</span><span class="n">setDevice</span><span class="p">(</span><span class="o">&</span><span class="n">file</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;">Making a member variable read from a function-local iodevice sounds dangerous to me. If you use d->fstream from a method not called by this one, you get a crash.
Maybe it would be better to pass the stream to the 2 methods that need it, rather than having it as a member variable? Or making file a member too.
Well, I guess that's not needed, it just seems fragile to have a member that is only usable when called from within one method.
==
A 2nd issue: what should be the encoding of the QTextStream? i.e. which encoding does the file use?</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;">I (think that I) fix it in the next patch, putting in NetRCPrivate those methods that don't have to be public (getMachinePart(), getMacdefPart() and extract() ), and leaving public only lookup(), reload() and parse(). I even thought about putting parse() in NetRCPrivate, but I left it protected in NetRC.
This way, I think there's no danger. The usual way of calling this class is calling lookup(), which calls parse() (and it calls the rest of the methods).
==
2nd issue: I don't know what the encoding of the QTextStream should be. </pre>
<br />
<p>- David</p>
<br />
<p>On July 21st, 2013, 11:41 p.m. UTC, David Gil Oliva wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By David Gil Oliva.</div>
<p style="color: grey;"><i>Updated July 21, 2013, 11:41 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;">Port away from kde_file.h in AuthInfo (KIO)
I have tried not to touch much code, but I finally rewrote some parts to make them easier to understand.</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;">It builds. I have tested the part of the code not related to loginMap with a little program and .netrc sample files, to check whether it correctly parses the information.</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>staging/kio/src/core/authinfo.h <span style="color: grey">(d6415b2f2e9ccec7c3e046f569fb44dbbc879d6b)</span></li>
<li>staging/kio/src/core/authinfo.cpp <span style="color: grey">(65ebacf84e989f19f1b896c596a6b24185c67447)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111636/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>