Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-04-10 Thread Alexander Richardson

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

(Updated April 10, 2014, 1:14 p.m.)


Status
--

This change has been marked as submitted.


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 1d17874f0da428bd34ea85ee98683f6fef620c81 
  src/lib/util/config-getgrouplist.h.cmake PRE-CREATION 
  src/lib/util/kuser_unix.cpp d1f7381a1e1d10fa1fdcd4b498a8ff007b8789e8 

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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-04-10 Thread Commit Hook

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


This review has been submitted with commit 
801544210718d01ef535047482acdd49c53175d6 by Alex Richardson to branch master.

- Commit Hook


On April 8, 2014, 8:30 a.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116881/
 ---
 
 (Updated April 8, 2014, 8:30 a.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 1d17874f0da428bd34ea85ee98683f6fef620c81 
   src/lib/util/config-getgrouplist.h.cmake PRE-CREATION 
   src/lib/util/kuser_unix.cpp d1f7381a1e1d10fa1fdcd4b498a8ff007b8789e8 
 
 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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-04-08 Thread Alexander Richardson

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

(Updated April 8, 2014, 10:30 a.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 (updated)
-

  src/lib/CMakeLists.txt 1d17874f0da428bd34ea85ee98683f6fef620c81 
  src/lib/util/config-getgrouplist.h.cmake PRE-CREATION 
  src/lib/util/kuser_unix.cpp d1f7381a1e1d10fa1fdcd4b498a8ff007b8789e8 

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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-04-08 Thread Michael Pyne

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

Ship it!


Ship It!

- Michael Pyne


On April 8, 2014, 8:30 a.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116881/
 ---
 
 (Updated April 8, 2014, 8:30 a.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 1d17874f0da428bd34ea85ee98683f6fef620c81 
   src/lib/util/config-getgrouplist.h.cmake PRE-CREATION 
   src/lib/util/kuser_unix.cpp d1f7381a1e1d10fa1fdcd4b498a8ff007b8789e8 
 
 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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-04-07 Thread Kevin Ottens

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


Any news?

- Kevin Ottens


On March 18, 2014, 8: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, 8: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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-04-07 Thread Alexander Richardson


 On April 7, 2014, 5:24 p.m., Kevin Ottens wrote:
  Any news?

Still wondering whether I should always use getgrouplist since AFAIK no 
platform without it is supported (yet) or should keep the fallback code.


- Alexander


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


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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-04-07 Thread Michael Pyne


 On April 7, 2014, 3:24 p.m., Kevin Ottens wrote:
  Any news?
 
 Alexander Richardson wrote:
 Still wondering whether I should always use getgrouplist since AFAIK no 
 platform without it is supported (yet) or should keep the fallback code.

Flip a coin if you have too. :)

As long as the decision you make is not actually *bad* then it's better to make 
a poor decision now than an ideal solution 3 weeks after we ship. We hopped on 
the Worse is Better ship when we latched up to C++ after all. ;)


- Michael


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


On March 18, 2014, 8: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, 8: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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-03-19 Thread Alexander Richardson


 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


Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-03-18 Thread Alexander Richardson

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

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


Group list before
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/03/18/37d6d8ca-f33a-4345-873c-2e4be0daba00__grouplist_getgrent_old.txt
New group list (with getgrouplist())
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/03/18/2cdf3e77-cd1e-4bfc-b49b-646947bf79c1__grouplist_getgrouplist.txt
New Group list (with getgrent())
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/03/18/ecef5bf6-b8bd-4675-a6e2-838f23615037__grouplist_getgrent_new.txt
 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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-03-18 Thread Alexander Richardson

---
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.


Changes
---

removed duplicate files (reviewboard showed they weren't there yet)


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 (updated)


 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


Re: Review Request 116881: Fix KUser::groups() and KUser::groupNames() on UNIX

2014-03-18 Thread Michael Pyne

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


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.


src/lib/util/kuser_unix.cpp
https://git.reviewboard.kde.org/r/116881/#comment37549

Same comment about callback as from my other review.



src/lib/util/kuser_unix.cpp
https://git.reviewboard.kde.org/r/116881/#comment37548

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?


- Michael Pyne


On March 18, 2014, 8: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, 8: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