<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://svn.reviewboard.kde.org/r/5775/">http://svn.reviewboard.kde.org/r/5775/</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://svn.reviewboard.kde.org/r/5775/diff/2/?file=40688#file40688line2699" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kdecore/localization/klocale_kde.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">QString KLocalePrivate::languageCodeToName(const QString &language)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2699</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="n">QStringList</span><span class="o">::</span><span class="n">ConstIterator</span> <span class="n">it</span> <span class="o">=</span> <span class="n">paths</span><span class="p">.</span><span class="n">begin</span><span class="p">()<span class="hl">;</span></span><span class="hl"> </span><span class="n"><span class="hl">it</span></span><span class="hl"> </span><span class="o"><span class="hl">!</span>=</span> <span class="n">paths</span><span class="p">.</span><span class="n">end</span><span class="p">();</span> <span class="o">++</span><span class="n">it</span><span class="p">)</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">2699</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="n">QStringList</span><span class="o">::</span><span class="n">ConstIterator</span> <span class="n">it</span> <span class="o">=</span> <span class="n">paths</span><span class="p">.</span><span class="n">begin</span><span class="p">()<span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">pathsEnd</span></span><span class="hl"> </span><span class="o">=</span> <span class="n">paths</span><span class="p">.</span><span class="n">end</span><span class="p">();</span> <span class="n"><span class="hl">it</span></span><span class="hl"> </span><span class="o"><span class="hl">!=</span></span><span class="hl"> </span><span class="n"><span class="hl">pathsEnd</span></span><span class="p"><span class="hl">;</span></span><span class="hl"> </span><span class="o">++</span><span class="n">it</span><span class="p">)</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;">The iterators here should be changed to constBegin() and constEnd(). Actually, there are many places in kdelibs that could benefit from such change as well... Without including the cases of non-const iterators, I get 185 hits when i execute the command below in kdelibs:
find -name "*.cpp" -exec grep --color -Hn "for" {} \; | grep "constEnd" | wc -l
</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://svn.reviewboard.kde.org/r/5775/diff/2/?file=40689#file40689line419" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kdecore/network/netsupp.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">static int inet_lookup(const char *name, int portnum, int protonum,</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">419</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">free</span><span class="p">(</span><span class="n">q</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;">Actually, the better fix here would simply be to move the code block that allocates and checks q for NULL below the "if (h == NULL)" block.</pre>
</div>
<br />
<p>- Dawit</p>
<br />
<p>On November 6th, 2010, 10:35 p.m., Jaime Torres wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 kdelibs.</div>
<div>By Jaime Torres.</div>
<p style="color: grey;"><i>Updated 2010-11-06 22:35:06</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;">As I do not know if that kind of patches are allowed in the freeze period, I ask for them together. I'll submit them individually.
1. ktimezone. Include a comment with the real use of refCount.
2. klocale_kde. From 469 queries to paths.end() to 1 (from 0.01% to 0% in callgrind)
3. netsupp. Remove a memory leak.
4. ksharedDataCache. Change the obsolete usleep (since 2001 or before) to nanosleep.</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;">No regressions in the unit tests.
Working with them.</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>/trunk/KDE/kdelibs/kdecore/date/ktimezone.cpp <span style="color: grey">(1193582)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/localization/klocale_kde.cpp <span style="color: grey">(1193582)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/network/netsupp.cpp <span style="color: grey">(1193582)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/util/kshareddatacache.cpp <span style="color: grey">(1193582)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5775/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>