<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="https://git.reviewboard.kde.org/r/117078/">https://git.reviewboard.kde.org/r/117078/</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="https://git.reviewboard.kde.org/r/117078/diff/2/?file=257291#file257291line1618" style="color: black; font-weight: bold; text-decoration: underline;">src/widgets/kpropertiesdialog.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; ">KFilePermissionsPropsPlugin::KFilePermissionsPropsPlugin(KPropertiesDialog *_props)</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">1617</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="kt">bool</span> <span class="n">IamRoot</span> <span class="o">=</span> <span class="n">myself</span><span class="p">.</span><span class="n">isSuperUser</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;">camelCase.
I would probably just name it as the function is named: "isSuperUser"</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="https://git.reviewboard.kde.org/r/117078/diff/2/?file=257291#file257291line3444" style="color: black; font-weight: bold; text-decoration: underline;">src/widgets/kpropertiesdialog.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; ">void KDesktopPropsPlugin::slotAdvanced()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3442</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setpwent</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">3411</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Q_FOREACH</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span><span class="o">&</span> <span class="n">userName</span><span class="p">,</span> <span class="n">userNames</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;">If you change it anyway, you might as well go for C++11:
for(const QString& userName : userNames) {...}
Just a matter of taste though. Feel free to ignore :)</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="https://git.reviewboard.kde.org/r/117078/diff/2/?file=257292#file257292line1555" style="color: black; font-weight: bold; text-decoration: underline;">src/widgets/kurlcompletion.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 bool expandTilde(QString &text)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1544</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">user</span><span class="p">.</span><span class="n">isEmpty</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">1542</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">user<span class="hl">Name</span></span><span class="p">.</span><span class="n">isEmpty</span><span class="p">())</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1545</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dir</span> <span class="o">=</span> <span class="n">QDir</span><span class="o">::</span><span class="n">homePath</span><span class="p">();</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1543</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dir</span> <span class="o">=</span> <span class="n">QDir</span><span class="o">::</span><span class="n">homePath</span><span class="p">();</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1546</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1544</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1547</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// ~user is replaced with the dir from passwd</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1545</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// ~user is replaced with the dir from passwd</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1548</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">//</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1546</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">//</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1549</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">else</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1547</font></th>
<td bgcolor="#ffffff" 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>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1550</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">struct</span> <span class="n">passwd</span> <span class="o">*</span><span class="n">pw</span> <span class="o">=</span> <span class="o">::</span><span class="n">getpwnam</span><span class="p">(</span><span class="n">user</span><span class="p">.</span><span class="n">toLocal8Bit</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">1548</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KUser</span> <span class="n">user</span><span class="p">(</span><span class="n">userName</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1551</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1549</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dir</span> <span class="o">=</span> <span class="n">user</span><span class="p">.</span><span class="n">homeDir</span><span class="p">();</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1552</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">pw</span><span class="p">)</span> <span class="p">{</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>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1553</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">dir</span> <span class="o">=</span> <span class="n">QFile</span><span class="o">::</span><span class="n">decodeName</span><span class="p">(</span><span class="n">pw</span><span class="o">-></span><span class="n">pw_dir</span><span class="p">);</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>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1554</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</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>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1555</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1556</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="o">::</span><span class="n">endpwent</span><span class="p">();</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>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1557</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1550</font></th>
<td bgcolor="#ffffff" 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;">Now it looks like either case (if and else) just return the users's home dir. You new code is certainly doing that. I don't know exactly what the old case was doing here.
What was it doing?</pre>
</div>
<br />
<p>- Mark Gaiser</p>
<br />
<p>On March 26th, 2014, 11:16 a.m. UTC, Alexander Richardson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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 Alexander Richardson.</div>
<p style="color: grey;"><i>Updated March 26, 2014, 11:16 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</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;">A series of commits (can submit separately if necessary):
---
Add S_IXUSR, IRUSR, etc. definitions for Windows
---
Add KIOPrivate::isProcessAlive() and KIOPrivate::sendTerminateSignal()
kill() does not work on Windows, therefore a WIN32 implementation is needed
in order to compile slave.cpp
---
Add KIOPrivate::symlinkTarget() to avoid directly calling readlink()
This allows KFileItem to be compiled on Windows.
The Windows implementation requires at least Windows Vista because
GetFinalPathNameByHandleW is not available in earlier version. Windows XP
support ends on the 8th of April so it should be okay to drop support for
it. If XP support is deemed necessary GetFinalPathNameByHandleW can be
loaded dynamically using GetProcAddress but that is not part of this patch.
Fix windows build of tests
---
Add KIOPrivate::changeOwnership() to avoid directly calling chown()
Additionally use KUserId/KGroupId instead of uid_t/gid_t.
This allows compiling chmodjob.cpp on Windows.
KIOPrivate::changeOwnership is a stub on Windows. However, it has always
been like that with the kdewin chmod() implementation, so this is not a
regression from kdelibs4.
---
Add KIOPrivate::createSymlink() to avoid using symlink() directly
---
Minor Windows compile fixes
---
Use KUser in KPropertiesDialog
This means no more need for getpwent(), etc -> works on Windows
---
Export the KIOPrivate functions
---
Fix kio_http build on Windows
---
No longer use uid_t/gid_t in kio_file
---
Allow compiling kurlcompletion.cpp on windows
---
Add a fake QT_LSTAT for Windows
This just calls QT_STAT and adds QT_STAT_LNK flag to st_mode if it is a
symlink
---
Use KUser in kpropertiesdialog.cpp -> no more getgrouplist
</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;">compiles, tests still the same as before (i.e. not passing)</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>autotests/fileundomanagertest.cpp <span style="color: grey">(3f209f89cc0e2ac48d8eaef7ee73ec18abca9a4c)</span></li>
<li>autotests/globaltest.cpp <span style="color: grey">(4367e53b44e077c566316081e21f429ac15b74a0)</span></li>
<li>autotests/kdirlistertest.cpp <span style="color: grey">(10a970416b8874f1e136b460d05378f8f3a86810)</span></li>
<li>autotests/kfileitemtest.h <span style="color: grey">(dc1808e726cf6af1605cfda520c7df3832225cc1)</span></li>
<li>autotests/kfileitemtest.cpp <span style="color: grey">(38bd87f4e9facd8e52e9e5fbd98b16011c866b5a)</span></li>
<li>autotests/kiotesthelper.h <span style="color: grey">(eb9f0f3019deb63506c2a173d700b78daa95ae10)</span></li>
<li>src/core/CMakeLists.txt <span style="color: grey">(d897e370baedbe06b267934c123acee7a149adff)</span></li>
<li>src/core/chmodjob.cpp <span style="color: grey">(271869bc2a643d715670560b7920efdbc948e560)</span></li>
<li>src/core/job_error.cpp <span style="color: grey">(1ec28c5d9ebdbc2045743c566f473c8cfa02e1bd)</span></li>
<li>src/core/kfileitem.cpp <span style="color: grey">(7364f87257b5d7dfb760b1c6e5b5d04e1d15a19d)</span></li>
<li>src/core/kioglobal_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/kioglobal_p_unix.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/kioglobal_p_win.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/slave.cpp <span style="color: grey">(787ffcf3cc97a73fb29c2172ed6b8df19ac016fc)</span></li>
<li>src/ioslaves/file/file.h <span style="color: grey">(6477df7cf0d26bf4f581151e1ce8e6c1115a221c)</span></li>
<li>src/ioslaves/file/file.cpp <span style="color: grey">(6387d7f29df2c0a26952be5255438361c37ed082)</span></li>
<li>src/ioslaves/file/file_win.cpp <span style="color: grey">(b0e433e5438e3c45f2f021bf073cb3cca8f4f923)</span></li>
<li>src/ioslaves/ftp/ftp.cpp <span style="color: grey">(49037701772f88e9a96bc3110f1f04fccb810ca8)</span></li>
<li>src/ioslaves/http/http.cpp <span style="color: grey">(3158628af2dc8ee88a0ac0d968748d6b50a8da33)</span></li>
<li>src/widgets/CMakeLists.txt <span style="color: grey">(61e4db3566bad08baaa2e7e90b862ddfc8b957f7)</span></li>
<li>src/widgets/config-getgrouplist.h.cmake <span style="color: grey">(6847a19d60be4eb5c2b65fb86258f7368848e6ab)</span></li>
<li>src/widgets/getgrouplist-fake.c <span style="color: grey">(dbe77067371dcedb80cea684fb3cd5f42ed72805)</span></li>
<li>src/widgets/kpropertiesdialog.cpp <span style="color: grey">(a3011a8adac98287bcada281dfc50b27e1af8fa0)</span></li>
<li>src/widgets/kurlcompletion.cpp <span style="color: grey">(3f309257c187358de0fd66f9d67f09a712fdf7d6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117078/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>