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