Handle NULL slots & bad spec; some light refactoring

Lib/test/test_capi/test_module.py

@@ -14,13 +14,26 @@ class FakeSpec:
     'Py_mod_traverse', 'Py_mod_clear', 'Py_mod_free', 'Py_mod_token',
 )
 
+# The C functions used by this module are in:
+#   Modules/_testcapi/module.c
 
 class TestModFromSlotsAndSpec(unittest.TestCase):
     def test_empty(self):
         mod = _testcapi.module_from_slots_empty(FakeSpec())
         self.assertIsInstance(mod, types.ModuleType)
         self.assertEqual(mod.__name__, 'testmod')
 
+    def test_null_slots(self):
+        with self.assertRaises(SystemError):
+            _testcapi.module_from_slots_null(FakeSpec())
+
+    def test_none_spec(self):
+        # The spec currently must contain a name
+        with self.assertRaises(AttributeError):
+            _testcapi.module_from_slots_empty(None)
+        with self.assertRaises(AttributeError):
+            _testcapi.module_from_slots_name(None)
+
     def test_name(self):
         # Py_mod_name (and PyModuleDef.m_name) are currently ignored when
         # spec is given.

Modules/_testcapi/module.c

@@ -3,6 +3,10 @@
 
 // Test PyModule_* API
 
+/* unittest Cases that use these functions are in:
+ * Lib/test/test_capi/test_module.py
+ */
+
 static PyObject *
 module_from_slots_empty(PyObject *self, PyObject *spec)
 {
@@ -12,6 +16,12 @@ module_from_slots_empty(PyObject *self, PyObject *spec)
     return PyModule_FromSlotsAndSpec(slots, spec);
 }
 
+static PyObject *
+module_from_slots_null(PyObject *self, PyObject *spec)
+{
+    return PyModule_FromSlotsAndSpec(NULL, spec);
+}
+
 static PyObject *
 module_from_slots_name(PyObject *self, PyObject *spec)
 {
@@ -242,6 +252,7 @@ module_from_def_slot(PyObject *self, PyObject *spec)
 
 static PyMethodDef test_methods[] = {
     {"module_from_slots_empty", module_from_slots_empty, METH_O},
+    {"module_from_slots_null", module_from_slots_null, METH_O},
     {"module_from_slots_name", module_from_slots_name, METH_O},
     {"module_from_slots_doc", module_from_slots_doc, METH_O},
     {"module_from_slots_size", module_from_slots_size, METH_O},

Objects/moduleobject.c

@@ -629,30 +629,19 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio
 PyObject *
 PyModule_FromSlotsAndSpec(PyModuleDef_Slot *slots, PyObject *spec)
 {
-    PyObject *result = NULL;
     if (!slots) {
-        PyErr_BadArgument();
-    }
-    PyObject *nameobj = PyObject_GetAttrString(spec, "name");
-    if (nameobj == NULL) {
-        goto finally;
-    }
-    const char *name = PyUnicode_AsUTF8(nameobj);
-    if (name == NULL) {
-        goto finally;
+        PyErr_Format(
+            PyExc_SystemError,
+            "PyModule_FromSlotsAndSpec called with NULL slots");
+        return NULL;
     }
-
     // Fill in enough of a PyModuleDef to pass to common machinery
     PyModuleDef def_like = {.m_slots = slots};
 
-    result = module_from_def_and_spec(&def_like, spec, PYTHON_API_VERSION,
-                                      NULL);
-finally:
-    Py_XDECREF(nameobj);
-    return result;
+    return module_from_def_and_spec(&def_like, spec, PYTHON_API_VERSION,
+                                    NULL);
 }
 
-
 #ifdef Py_GIL_DISABLED
 int
 PyUnstable_Module_SetGIL(PyObject *module, void *gil)