gh-110815: Improve tests for PyArg_ParseTupleAndKeywords() (GH-110817)

Author: serhiy-storchaka

Lib/test/test_capi/test_getargs.py

@@ -55,6 +55,8 @@
 LLONG_MIN = -2**63
 ULLONG_MAX = 2**64-1
 
+NULL = None
+
 class Index:
     def __index__(self):
         return 99
@@ -1160,6 +1162,27 @@ def test_parse_tuple_and_keywords(self):
         self.assertRaises(ValueError, _testcapi.parse_tuple_and_keywords,
                           (), {}, '', [42])
 
+    def test_basic(self):
+        parse = _testcapi.parse_tuple_and_keywords
+
+        self.assertEqual(parse((), {'a': 1}, 'O', ['a']), (1,))
+        self.assertEqual(parse((), {}, '|O', ['a']), (NULL,))
+        self.assertEqual(parse((1, 2), {}, 'OO', ['a', 'b']), (1, 2))
+        self.assertEqual(parse((1,), {'b': 2}, 'OO', ['a', 'b']), (1, 2))
+        self.assertEqual(parse((), {'a': 1, 'b': 2}, 'OO', ['a', 'b']), (1, 2))
+        self.assertEqual(parse((), {'b': 2}, '|OO', ['a', 'b']), (NULL, 2))
+
+        with self.assertRaisesRegex(TypeError,
+                "function missing required argument 'a'"):
+            parse((), {}, 'O', ['a'])
+        with self.assertRaisesRegex(TypeError,
+                "'b' is an invalid keyword argument"):
+            parse((), {'b': 1}, '|O', ['a'])
+        with self.assertRaisesRegex(TypeError,
+                fr"argument for function given by name \('a'\) "
+                fr"and position \(1\)"):
+            parse((1,), {'a': 2}, 'O|O', ['a', 'b'])
+
     def test_bad_use(self):
         # Test handling invalid format and keywords in
         # PyArg_ParseTupleAndKeywords()
@@ -1187,20 +1210,23 @@ def test_bad_use(self):
     def test_positional_only(self):
         parse = _testcapi.parse_tuple_and_keywords
 
-        parse((1, 2, 3), {}, 'OOO', ['', '', 'a'])
-        parse((1, 2), {'a': 3}, 'OOO', ['', '', 'a'])
+        self.assertEqual(parse((1, 2, 3), {}, 'OOO', ['', '', 'a']), (1, 2, 3))
+        self.assertEqual(parse((1, 2), {'a': 3}, 'OOO', ['', '', 'a']), (1, 2, 3))
         with self.assertRaisesRegex(TypeError,
                r'function takes at least 2 positional arguments \(1 given\)'):
             parse((1,), {'a': 3}, 'OOO', ['', '', 'a'])
-        parse((1,), {}, 'O|OO', ['', '', 'a'])
+        self.assertEqual(parse((1,), {}, 'O|OO', ['', '', 'a']),
+                         (1, NULL, NULL))
         with self.assertRaisesRegex(TypeError,
                r'function takes at least 1 positional argument \(0 given\)'):
             parse((), {}, 'O|OO', ['', '', 'a'])
-        parse((1, 2), {'a': 3}, 'OO$O', ['', '', 'a'])
+        self.assertEqual(parse((1, 2), {'a': 3}, 'OO$O', ['', '', 'a']),
+                         (1, 2, 3))
         with self.assertRaisesRegex(TypeError,
                r'function takes exactly 2 positional arguments \(1 given\)'):
             parse((1,), {'a': 3}, 'OO$O', ['', '', 'a'])
-        parse((1,), {}, 'O|O$O', ['', '', 'a'])
+        self.assertEqual(parse((1,), {}, 'O|O$O', ['', '', 'a']),
+                         (1, NULL, NULL))
         with self.assertRaisesRegex(TypeError,
                r'function takes at least 1 positional argument \(0 given\)'):
             parse((), {}, 'O|O$O', ['', '', 'a'])

Modules/_testcapi/getargs.c

@@ -13,9 +13,9 @@ parse_tuple_and_keywords(PyObject *self, PyObject *args)
     const char *sub_format;
     PyObject *sub_keywords;
 
-    double buffers[8][4]; /* double ensures alignment where necessary */
-    PyObject *converted[8];
-    char *keywords[8 + 1]; /* space for NULL at end */
+#define MAX_PARAMS 8
+    double buffers[MAX_PARAMS][4]; /* double ensures alignment where necessary */
+    char *keywords[MAX_PARAMS + 1]; /* space for NULL at end */
 
     PyObject *return_value = NULL;
 
@@ -35,41 +35,67 @@ parse_tuple_and_keywords(PyObject *self, PyObject *args)
     }
 
     memset(buffers, 0, sizeof(buffers));
-    memset(converted, 0, sizeof(converted));
     memset(keywords, 0, sizeof(keywords));
 
     Py_ssize_t size = PySequence_Fast_GET_SIZE(sub_keywords);
-    if (size > 8) {
+    if (size > MAX_PARAMS) {
         PyErr_SetString(PyExc_ValueError,
             "parse_tuple_and_keywords: too many keywords in sub_keywords");
         goto exit;
     }
 
     for (Py_ssize_t i = 0; i < size; i++) {
         PyObject *o = PySequence_Fast_GET_ITEM(sub_keywords, i);
-        if (!PyUnicode_FSConverter(o, (void *)(converted + i))) {
+        if (PyUnicode_Check(o)) {
+            keywords[i] = (char *)PyUnicode_AsUTF8(o);
+            if (keywords[i] == NULL) {
+                goto exit;
+            }
+        }
+        else if (PyBytes_Check(o)) {
+            keywords[i] = PyBytes_AS_STRING(o);
+        }
+        else {
             PyErr_Format(PyExc_ValueError,
                 "parse_tuple_and_keywords: "
-                "could not convert keywords[%zd] to narrow string", i);
+                "keywords must be str or bytes", i);
             goto exit;
         }
-        keywords[i] = PyBytes_AS_STRING(converted[i]);
     }
 
+    assert(MAX_PARAMS == 8);
     int result = PyArg_ParseTupleAndKeywords(sub_args, sub_kwargs,
         sub_format, keywords,
         buffers + 0, buffers + 1, buffers + 2, buffers + 3,
         buffers + 4, buffers + 5, buffers + 6, buffers + 7);
 
     if (result) {
-        return_value = Py_NewRef(Py_None);
+        int objects_only = 1;
+        for (const char *f = sub_format; *f; f++) {
+            if (Py_ISALNUM(*f) && strchr("OSUY", *f) == NULL) {
+                objects_only = 0;
+                break;
+            }
+        }
+        if (objects_only) {
+            return_value = PyTuple_New(size);
+            if (return_value == NULL) {
+                goto exit;
+            }
+            for (Py_ssize_t i = 0; i < size; i++) {
+                PyObject *arg = *(PyObject **)(buffers + i);
+                if (arg == NULL) {
+                    arg = Py_None;
+                }
+                PyTuple_SET_ITEM(return_value, i, Py_NewRef(arg));
+            }
+        }
+        else {
+            return_value = Py_NewRef(Py_None);
+        }
     }
 
 exit:
-    size = sizeof(converted) / sizeof(converted[0]);
-    for (Py_ssize_t i = 0; i < size; i++) {
-        Py_XDECREF(converted[i]);
-    }
     return return_value;
 }