Bug 89646

Summary:	kpropertiesdialog should use getgrouplist() instead of enumerating all groups
Product:	[Frameworks and Libraries] kio	Reporter:	andreas
Component:	kfile	Assignee:	David Faure <faure>
Status:	RESOLVED FIXED
Severity:	normal	CC:	faure
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Conectiva RPMs
OS:	Linux
Latest Commit:		Version Fixed In:
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