Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX
Alexander Richardson
arichardson.kde at googlemail.com
Wed Mar 19 06:55:40 UTC 2014
> On March 19, 2014, 4:11 a.m., Michael Pyne wrote:
> > I think I'd argue against bothering with getgrouplist at all if we have to maintain a backup to it either way, it just makes the code messier. But I'll leave that part up to you (maybe it's that much faster).
> >
> > Still a couple of other comments though.
Well I think it should be available everywhere: Linux, Mac, *BSD. Maybe non-glibc Linux systems haven't got it. I just left the fallback because we basically had that code already.
Maybe leave it there inside an #if 0 block should the need arise?
> On March 19, 2014, 4:11 a.m., Michael Pyne wrote:
> > src/lib/util/kuser_unix.cpp, line 197
> > <https://git.reviewboard.kde.org/r/116881/diff/1/?file=255229#file255229line197>
> >
> > Same comment about "callback" as from my other review.
I read that the compiler won't inline std::function, will check that later.
> On March 19, 2014, 4:11 a.m., Michael Pyne wrote:
> > src/lib/util/kuser_unix.cpp, line 217
> > <https://git.reviewboard.kde.org/r/116881/diff/1/?file=255229#file255229line217>
> >
> > Not sure what you mean with the comment here. Shouldn't gid_buffer[i] be just as safe (if not more) than obtaining the gid_t* and then dereferencing that as an array?
I only used reserve() and not resize(), but I guess that was a premature optimization.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116881/#review53383
-----------------------------------------------------------
On March 18, 2014, 9:14 p.m., Alexander Richardson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116881/
> -----------------------------------------------------------
>
> (Updated March 18, 2014, 9:14 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kcoreaddons
>
>
> Description
> -------
>
> Fix KUser::groups() and KUser::groupNames() on UNIX
>
> If available we use getgrouplist() which returns all group IDs.
> Otherwise we fall back to using getgrent() and checking whether gr_mem
> contains the user. For some reason gr_mem doesn't usally contain the
> users primary gid, so we add the corresponding struct group for that gid
> as well.
>
>
> Diffs
> -----
>
> src/lib/CMakeLists.txt 73f28a8a3acda8449a3aee3b025e2b165722b998
> src/lib/util/config-getgrouplist.h.cmake PRE-CREATION
> src/lib/util/kuser_unix.cpp 0e0720dbb614015d6d568b39da3cab77cece76a8
>
> Diff: https://git.reviewboard.kde.org/r/116881/diff/
>
>
> Testing
> -------
>
> Returns more groups now, should fix KUserTest::testKUser() on build.kde.org
>
>
> File Attachments
> ----------------
>
> old user->groups result (getgrent)
> https://git.reviewboard.kde.org/media/uploaded/files/2014/03/18/86c5e986-8098-4c50-809c-bc3e7851447a__grouplist_getgrent_old.txt
> new user->groups result (getgrouplist)
> https://git.reviewboard.kde.org/media/uploaded/files/2014/03/18/59f4ecda-5c1d-4bdc-a8ec-850de761e3a3__grouplist_getgrouplist.txt
> new user->groups result (getgrent + current gid)
> https://git.reviewboard.kde.org/media/uploaded/files/2014/03/18/39d0172d-f6c8-400e-8af0-0c4aace8f7a1__grouplist_getgrent_new.txt
>
>
> Thanks,
>
> Alexander Richardson
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140319/f7baad20/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list