<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/116798/">https://git.reviewboard.kde.org/r/116798/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 17th, 2014, 4:28 p.m. CET, <b>Aurélien Gâteau</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/116798/diff/4/?file=254169#file254169line216" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/util/kuser.h</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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">216</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">explicit</span> <span class="nf">KUser</span><span class="p">(</span><span class="n">KUserId</span> <span class="n">uid</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;">I know the review has already been submitted, but shouldn't this be "const KUserId &uid" instead of "KUserId uid"?
Same for KUserGroup.</pre>
</blockquote>
<p>On March 18th, 2014, 1:15 p.m. CET, <b>Alexander Richardson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It is just a uint16/uint32 on Linux, I don't see an advantage in passing that by const reference. On Windows it only increments the refcount once too much (assuming the compiler isn't clever enough) since it is stored in the Private class anyway.</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;">OK, never mind then. Thanks for the feedback.</pre>
<br />
<p>- Aurélien</p>
<br />
<p>On March 16th, 2014, 10:55 p.m. CET, 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 16, 2014, 10:55 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kcoreaddons
</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;">This was all started in order to get KIO to compile on Windows (uses uid_t/gid_t, getpwent, getpwuid, getuid, etc.)
This patchset introduces KUserId/KGroupId which is a no-overhead wrapper around uid_t/gid_t and implicitly shares a SID on Windows.
Also introduced a maxCount paramter to all listing functions of KUser/KUserGroup so that the manual calls to getpwent() in KIO can be replaced
Finally added a unit test for KUser, KUserGroup, KUserId, KGroupId to ensure that these changes are safe
Windows only changes:
- KUser::homeDirectory() works properly, before it always returned the home directory of the current user
- KUser::faceIconPath() is now implemented on Windows
- Use scoped pointers everywhere -> no memory leaks (at least one was fixed)
This is actually a series of commits, if you think that is easier to review I can push them somewhere.
</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;">Wrote new unit test, passes on Linux and Windows, KIO is closer to compiling on windows</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/CMakeLists.txt <span style="color: grey">(30ac97822a77e4b12b0e81a4a5c93fe9a768d915)</span></li>
<li>autotests/kusertest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/util/kuser.h <span style="color: grey">(42c81156831d0269c89435c76c3572dcf2e085b5)</span></li>
<li>src/lib/util/kuser_unix.cpp <span style="color: grey">(aa2f9073015c7f9feb8f74e3646928d2fa2de007)</span></li>
<li>src/lib/util/kuser_win.cpp <span style="color: grey">(06759afa34ea7015a44cf9b38f541f944f8126d4)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/116798/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>