Bug 89646

Summary: kpropertiesdialog should use getgrouplist() instead of enumerating all groups
Product: [Unmaintained] kio Reporter: andreas
Component: kfileAssignee: David Faure <faure>
Status: RESOLVED FIXED    
Severity: normal CC: faure
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Conectiva RPMs   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: kde-groupenum.patch
kde-groupenum.patch

Description andreas 2004-09-16 19:29:33 UTC 
Version:           3.3.0 (using KDE KDE 3.3.0)
Installed from:    Conectiva RPMs
Compiler:          gcc-3.4.2 
OS:                Linux

kpropertiesdialog.cpp uses setgrent()/getgrent()/endgrent() to find out to which groups a specific user is member of. This ends up listing all available groups, and, although there is a limit, this can be expensive when a centralized authentication/authorization setup is in use.

In my case, for example, the machine is a member of a big windows 2000 forest and user and group information is retrieved via winbind.  The current code will enumerate all groups available in this setup up to 1000 (there are more). Since this enumeration is over the network, it takes a long time.

What happens is that whenever the user right-clicks on some icon and brings up the properties dialog box, KDE hangs for about 6 minutes while all these groups are being retrieved.

I think it would be better to use another way to fetch this information. In particular, glibc since version 2.2.4 has getgrouplist(2) which does this just fine and does not enumerate all groups. It should be used instead.
Comment 1 andreas 2004-09-16 19:36:50 UTC 
The particular piece of code in question:

kpropertiesdialog.cpp:1633
  setgrent();
  for (i=0; ((ge = getgrent()) != 0L) && (i < maxEntries); i++)
  {
    if (IamRoot)
      groupList += QString::fromLatin1(ge->gr_name);
    else
    {
      /* pick the groups to which the user belongs */
      char ** members = ge->gr_mem;
      char * member;
      while ((member = *members) != 0L) {
        if (strUser == member) {
          groupList += QString::fromLocal8Bit(ge->gr_name);
          break;
        }
        ++members;
      }
    }
  }
  endgrent();
Comment 2 Stephan Kulow 2004-09-22 17:45:26 UTC 
Got a patch?
Comment 3 andreas 2004-09-22 22:59:12 UTC 
Created attachment 7635 [details]
kde-groupenum.patch

I'm about to try this patch. I don't know if it works yet, please take a look.
Comment 4 andreas 2004-10-18 19:54:00 UTC 
Created attachment 7938 [details]
kde-groupenum.patch

Slightly modified version. I tested this and it gave quite a performance boost
as expected (38s vs. <1s more or less) in my testcase.
It still needs work, though, I'm not a good coder, just a problem spotter :)
realloc should be used, or some other qt/kde memory management function. The
first malloc is also never checked. I hope this is enough for some kde
developer to carry on, though.
Comment 5 David Faure 2009-03-06 12:51:42 UTC 
SVN commit 935835 by dfaure:

Use getgrouplist() instead of enumerating all groups with setgrent/getgrent, which can be really slow with centralized setups (e.g. winbind)
Based on patch by andreas at conectiva - thanks!  (code simplified and made more C++). Fix will be in 4.2.2.
BUG: 89646


 M  +17 -29    kpropertiesdialog.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=935835