Review Request 116883: Fix KUserGroup::users() and KUserGroup::userNames() on UNIX

[Milian Wolff]

> On March 19, 2014, 2:56 a.m., Michael Pyne wrote:
> > src/lib/util/kuser_unix.cpp, line 349
> > <https://git.reviewboard.kde.org/r/116883/diff/1/?file=255233#file255233line349>
> >
> >     Does this need to be a template, or would std::function be sufficient? Templates have a poor reputation amongst some of our devs so I wouldn't make them the first answer unless they cleanly fit the problem.
> 
> Alexander Richardson wrote:
>     I think std::function is much nicer as well, because it allows specifying the argument list. However I think the compiler won't inline the std::function. Probably not that important, since this code will rarely be called, but I'll check that first.

Not voting for anything, but Michael: "Templates have a poor reputation amongst some of our devs" - Then it's our job to educate these developers. std::function is also a template with additional type erasure on top. If at all, it's more "complicated" than passing a functor directly.

Considering that this function is internal (in a .cpp file), I don't see why this should be a std::function.


- Milian


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


On March 19, 2014, 8:20 a.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116883/
> -----------------------------------------------------------
> 
> (Updated March 19, 2014, 8:20 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> Fix KUserGroup::users() and KUserGroup::userNames() on UNIX
> 
> The gr_mem member of struct group does not contain the names of users
> where the primary group is equal to that group. In order to fix this we
> have to iterate over all users and check whether the primary gid matches
> the requested group. Unlike getgrouplist() there does not seem to be a
> function that performs this task for us.
> 
> 
> Diffs
> -----
> 
>   autotests/kusertest.cpp 369bd56cbba4554b04c03847b74fb023a426f5eb 
>   src/lib/util/kuser_unix.cpp 0e0720dbb614015d6d568b39da3cab77cece76a8 
> 
> Diff: https://git.reviewboard.kde.org/r/116883/diff/
> 
> 
> Testing
> -------
> 
> Listed groups seem to be correct now. Should fix Jenkins together with https://git.reviewboard.kde.org/r/116881/
> 
> 
> File Attachments
> ----------------
> 
> Before
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/03/18/871c8bc5-4631-4759-8103-eb7ac817d698__groupmembers_before.txt
> After
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/03/18/4295e719-acb8-495f-907b-5a93d2d8b64f__groupmembers_after.txt
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140319/04360f4e/attachment-0001.html>