[Python-checkins] r83088 - in python/branches/py3k: Lib/test/test_posix.py Misc/NEWS Modules/posixmodule.c

[ronald.oussoren]

Author: ronald.oussoren
Date: Fri Jul 23 15:53:51 2010
New Revision: 83088

Log:
This fixes issue7900 by adding code that deals
with the fact that getgroups(2) might return
more that MAX_GROUPS on OSX.

See the issue (and python-dev archives) for the
gory details. Summarized: OSX behaves rather oddly
and Apple says this is intentional.


Modified:
   python/branches/py3k/Lib/test/test_posix.py
   python/branches/py3k/Misc/NEWS
   python/branches/py3k/Modules/posixmodule.c

Modified: python/branches/py3k/Lib/test/test_posix.py
==============================================================================
--- python/branches/py3k/Lib/test/test_posix.py	(original)
+++ python/branches/py3k/Lib/test/test_posix.py	Fri Jul 23 15:53:51 2010
@@ -6,6 +6,7 @@
 posix = support.import_module('posix')
 
 import errno
+import sys
 import time
 import os
 import pwd
@@ -349,9 +350,59 @@
                 os.chdir(curdir)
                 support.rmtree(base_path)
 
+    def test_getgroups(self):
+        with os.popen('id -G') as idg:
+            groups = idg.read().strip()
+
+        if not groups:
+            raise unittest.SkipTest("need working 'id -G'")
+
+        self.assertEqual([int(x) for x in groups.split()], posix.getgroups())
+
+class PosixGroupsTester(unittest.TestCase):
+
+    def setUp(self):
+        if posix.getuid() != 0:
+            raise unittest.SkipTest("not enough privileges")
+        if not hasattr(posix, 'getgroups'):
+            raise unittest.SkipTest("need posix.getgroups")
+        if sys.platform == 'darwin':
+            raise unittest.SkipTest("getgroups(2) is broken on OSX")
+        self.saved_groups = posix.getgroups()
+
+    def tearDown(self):
+        if hasattr(posix, 'setgroups'):
+            posix.setgroups(self.saved_groups)
+        elif hasattr(posix, 'initgroups'):
+            name = pwd.getpwuid(posix.getuid()).pw_name
+            posix.initgroups(name, self.saved_groups[0])
+
+    @unittest.skipUnless(hasattr(posix, 'initgroups'),
+                         "test needs posix.initgroups()")
+    def test_initgroups(self):
+        # find missing group
+
+        groups = sorted(self.saved_groups)
+        for g1,g2 in zip(groups[:-1], groups[1:]):
+            g = g1 + 1
+            if g < g2:
+                break
+        else:
+            g = g2 + 1
+        name = pwd.getpwuid(posix.getuid()).pw_name
+        posix.initgroups(name, g)
+        self.assertIn(g, posix.getgroups())
+
+    @unittest.skipUnless(hasattr(posix, 'setgroups'),
+                         "test needs posix.setgroups()")
+    def test_setgroups(self):
+        for groups in [[0], range(16)]:
+            posix.setgroups(groups)
+            self.assertListEqual(groups, posix.getgroups())
+
 
 def test_main():
-    support.run_unittest(PosixTester)
+    support.run_unittest(PosixTester, PosixGroupsTester)
 
 if __name__ == '__main__':
     test_main()

Modified: python/branches/py3k/Misc/NEWS
==============================================================================
--- python/branches/py3k/Misc/NEWS	(original)
+++ python/branches/py3k/Misc/NEWS	Fri Jul 23 15:53:51 2010
@@ -1440,6 +1440,13 @@
 Extension Modules
 -----------------
 
+- Issue #7900: The getgroups(2) system call on MacOSX behaves rather oddly
+  compared to other unix systems. In particular, os.getgroups() does
+  not reflect any changes made using os.setgroups() but basicly always
+  returns the same information as the id command.
+
+  os.getgroups() can now return more than 16 groups on MacOSX.
+
 - Issue #6095: Make directory argument to os.listdir optional.
 
 - Issue #9277: Fix bug in struct.pack for bools in standard mode

Modified: python/branches/py3k/Modules/posixmodule.c
==============================================================================
--- python/branches/py3k/Modules/posixmodule.c	(original)
+++ python/branches/py3k/Modules/posixmodule.c	Fri Jul 23 15:53:51 2010
@@ -4166,17 +4166,49 @@
 #define MAX_GROUPS 64
 #endif
     gid_t grouplist[MAX_GROUPS];
+
+    /* On MacOSX getgroups(2) can return more than MAX_GROUPS results 
+     * This is a helper variable to store the intermediate result when
+     * that happens.
+     *
+     * To keep the code readable the OSX behaviour is unconditional,
+     * according to the POSIX spec this should be safe on all unix-y
+     * systems.
+     */
+    gid_t* alt_grouplist = grouplist;
     int n;
 
     n = getgroups(MAX_GROUPS, grouplist);
-    if (n < 0)
-        posix_error();
-    else {
-        result = PyList_New(n);
-        if (result != NULL) {
+    if (n < 0) {
+        if (errno == EINVAL) {
+            n = getgroups(0, NULL);
+            if (n == -1) {
+                return posix_error();
+            }
+            if (n == 0) {
+                /* Avoid malloc(0) */
+                alt_grouplist = grouplist;
+            } else {
+                alt_grouplist = PyMem_Malloc(n * sizeof(gid_t));
+                if (alt_grouplist == NULL) {
+                    errno = EINVAL;
+                    return posix_error();
+                }
+                n = getgroups(n, alt_grouplist);
+                if (n == -1) {
+                    PyMem_Free(alt_grouplist);
+                    return posix_error();
+                }
+            }
+        } else {
+            return posix_error();
+        }
+    }
+    result = PyList_New(n);
+    if (result != NULL) {
         int i;
         for (i = 0; i < n; ++i) {
-            PyObject *o = PyLong_FromLong((long)grouplist[i]);
+            PyObject *o = PyLong_FromLong((long)alt_grouplist[i]);
             if (o == NULL) {
             Py_DECREF(result);
             result = NULL;
@@ -4184,7 +4216,10 @@
             }
             PyList_SET_ITEM(result, i, o);
         }
-        }
+    }
+
+    if (alt_grouplist != grouplist) {
+        PyMem_Free(alt_grouplist);
     }
 
     return result;