On Thu, Aug 09, 2012 at 02:20:23PM +0200, Sumit Bose wrote:
Hi,
I would like to find a range/slice for Posix IDs based on a domain SID
on the IPA server the same way as sssd does. For this I need python
bindings for the murmurhash3() call. The attached patch adds them and a
small test.
bye,
Sumit
The code works as expected as is mostly OK, I would only like to see two
nitpicks fixed to make the code "more pythonic", see inline:
+static PyObject * py_murmurhash3(PyObject *module, PyObject *args)
+{
+ const char *key;
+ long key_len;
+ unsigned long seed;
+ uint32_t hash;
+
+ if (!PyArg_ParseTuple(args, "slk", &key, &key_len, &seed)) {
+ return NULL;
+ }
+
+ if (seed > UINT32_MAX || key_len > INT_MAX || key_len < INT_MIN ||
+ key_len > strlen(key) || seed > UINT32_MAX) {
I think we should first set an exception here before returning NULL,
something like:
PyErr_Format(PyExc_ValueError, "Invalid value\n");
+ return NULL;
+ }
+
+ hash = murmurhash3(key, key_len, seed);
+
+ return PyLong_FromUnsignedLong((unsigned long) hash);
+}
+class PySssMurmurImport(unittest.TestCase):
+ def setUp(self):
+ " Make sure we load the in-tree module "
+ self.system_path = sys.path[:]
+ sys.path = [ MODPATH ]
+
+ def tearDown(self):
+ " Restore the system path "
+ sys.path = self.system_path
+
+ def testImport(self):
+ " Import the module and assert it comes from tree "
+ try:
+ import pysss_murmur
+ except ImportError, e:
+ print >>sys.stderr, "Could not load the pysss_murmur module.
Please check if it is compiled"
+ raise e
+ self.assertEqual(pysss_murmur.__file__, MODPATH + "/pysss_murmur.so")
+
+class PySssMurmurTest(unittest.TestCase):
+ def testExpectedHash(self):
+ hash =
pysss_murmur.murmurhash3("S-1-5-21-2153326666-2176343378-3404031434", 41,
0xdeadbeef)
+ assert hash == 93103853
I think that using self.assertEqual(hash, 93103853) is more expected
here.
+
+if __name__ == "__main__":
+ error = 0
+
+ suite = unittest.TestLoader().loadTestsFromTestCase(PySssMurmurImport)
+ res = unittest.TextTestRunner().run(suite)
+ if not res.wasSuccessful():
+ error |= 0x1
+ # need to bail out here because pyhbac could not be imported
^^^ copy-n-paste bug :-)
+ sys.exit(error)
+
+ # import the pyhbac module into the global namespace, but make sure it's
^^^ also here
+ # the one in tree
+ sys.path.insert(0, MODPATH)
+ import pysss_murmur
+
+ suite = unittest.TestLoader().loadTestsFromTestCase(PySssMurmurTest)
+ res = unittest.TextTestRunner().run(suite)
+ if not res.wasSuccessful():
+ error |= 0x2
+
+ sys.exit(error)
The rest looks good to me.