Review Request 116798: Rewrite KUser/KUserGroup Widows implementation + introduce KUserId/KGroupId

[Alexander Richardson]

> On March 17, 2014, 4:28 p.m., Aurélien Gâteau wrote:
> > src/lib/util/kuser.h, line 216
> > <https://git.reviewboard.kde.org/r/116798/diff/4/?file=254169#file254169line216>
> >
> >     I know the review has already been submitted, but shouldn't this be "const KUserId &uid" instead of "KUserId uid"?
> >     
> >     Same for KUserGroup.

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.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116798/#review53177
-----------------------------------------------------------


On March 16, 2014, 10:55 p.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116798/
> -----------------------------------------------------------
> 
> (Updated March 16, 2014, 10:55 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 30ac97822a77e4b12b0e81a4a5c93fe9a768d915 
>   autotests/kusertest.cpp PRE-CREATION 
>   src/lib/util/kuser.h 42c81156831d0269c89435c76c3572dcf2e085b5 
>   src/lib/util/kuser_unix.cpp aa2f9073015c7f9feb8f74e3646928d2fa2de007 
>   src/lib/util/kuser_win.cpp 06759afa34ea7015a44cf9b38f541f944f8126d4 
> 
> Diff: https://git.reviewboard.kde.org/r/116798/diff/
> 
> 
> Testing
> -------
> 
> Wrote new unit test, passes on Linux and Windows, KIO is closer to compiling on windows
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140318/82e6de07/attachment.html>