> 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
