Bug 89646 - kpropertiesdialog should use getgrouplist() instead of enumerating all groups
Summary: kpropertiesdialog should use getgrouplist() instead of enumerating all groups
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Unclassified
Component: kfile (show other bugs)
Version: unspecified
Platform: Conectiva RPMs Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-09-16 19:29 UTC by andreas
Modified: 2009-03-06 12:51 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
kde-groupenum.patch (1.73 KB, patch)
2004-09-22 22:59 UTC, andreas
Details
kde-groupenum.patch (1.75 KB, patch)
2004-10-18 19:54 UTC, andreas
Details

Note You need to log in before you can comment on or make changes to this bug.
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