From 2f0baa6b15c8d8faab6018b1800c594aa37b07d1 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 10 Oct 2023 11:09:08 +0300 Subject: [PATCH 1/4] gh-84489: C API: Add tests for Py_BuildValue() --- Lib/test/test_capi/test_misc.py | 54 +++++++++++++++++++++++++++++++++ Modules/_testcapimodule.c | 20 ++++++++++++ 2 files changed, 74 insertions(+) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 5ece213e7b2363b..f5bb37fb85d3fd5 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -298,6 +298,60 @@ def test_getitem_with_error(self): # test _Py_CheckFunctionResult() instead. self.assertIn('returned a result with an exception set', err) + def test_buildvalue(self): + """Test Py_BuildValue()""" + buildvalue = _testcapi.py_buildvalue + self.assertEqual(buildvalue(''), None) + self.assertEqual(buildvalue('()'), ()) + self.assertEqual(buildvalue('[]'), []) + self.assertEqual(buildvalue('{}'), {}) + self.assertEqual(buildvalue('()[]{}'), ((), [], {})) + self.assertEqual(buildvalue('O', 1), 1) + self.assertEqual(buildvalue('(O)', 1), (1,)) + self.assertEqual(buildvalue('[O]', 1), [1]) + self.assertRaises(SystemError, buildvalue, '{O}', 1) + self.assertEqual(buildvalue('OO', 1, 2), (1, 2)) + self.assertEqual(buildvalue('(OO)', 1, 2), (1, 2)) + self.assertEqual(buildvalue('[OO]', 1, 2), [1, 2]) + self.assertEqual(buildvalue('{OO}', 1, 2), {1: 2}) + self.assertEqual(buildvalue('{OOOO}', 1, 2, 3, 4), {1: 2, 3: 4}) + self.assertEqual(buildvalue('((O))', 1), ((1,),)) + self.assertEqual(buildvalue('((OO))', 1, 2), ((1, 2),)) + + self.assertEqual(buildvalue(' \t,:'), None) + self.assertEqual(buildvalue(' O ', 1), 1) + self.assertEqual(buildvalue('\tO\t', 1), 1) + self.assertEqual(buildvalue('O,O', 1, 2), (1, 2)) + self.assertEqual(buildvalue('O, O', 1, 2), (1, 2)) + self.assertEqual(buildvalue('O,\tO', 1, 2), (1, 2)) + self.assertEqual(buildvalue('O O', 1, 2), (1, 2)) + self.assertEqual(buildvalue('O\tO', 1, 2), (1, 2)) + self.assertEqual(buildvalue('(O,O)', 1, 2), (1, 2)) + self.assertEqual(buildvalue('(O, O)', 1, 2), (1, 2)) + self.assertEqual(buildvalue(' ( O O) ', 1, 2), (1, 2)) + self.assertEqual(buildvalue('\t(\tO\tO)\t', 1, 2), (1, 2)) + self.assertEqual(buildvalue('[O,O]', 1, 2), [1, 2]) + self.assertEqual(buildvalue('[O, O]', 1, 2), [1, 2]) + self.assertEqual(buildvalue(' [ O O] ', 1, 2), [1, 2]) + self.assertEqual(buildvalue('{O:O}', 1, 2), {1: 2}) + self.assertEqual(buildvalue('{O:O,O:O}', 1, 2, 3, 4), {1: 2, 3: 4}) + self.assertEqual(buildvalue('{O: O, O: O}', 1, 2, 3, 4), {1: 2, 3: 4}) + self.assertEqual(buildvalue(' { O O O O} ', 1, 2, 3, 4), {1: 2, 3: 4}) + self.assertEqual(buildvalue('\t{\tO\tO\tO\tO}\t', 1, 2, 3, 4), {1: 2, 3: 4}) + + self.assertRaises(SystemError, buildvalue, 'O', NULL) + self.assertRaises(SystemError, buildvalue, '(O)', NULL) + self.assertRaises(SystemError, buildvalue, '[O]', NULL) + self.assertRaises(SystemError, buildvalue, '{O}', NULL) + self.assertRaises(SystemError, buildvalue, 'OO', 1, NULL) + self.assertRaises(SystemError, buildvalue, 'OO', NULL, 2) + self.assertRaises(SystemError, buildvalue, '(OO)', 1, NULL) + self.assertRaises(SystemError, buildvalue, '(OO)', NULL, 2) + self.assertRaises(SystemError, buildvalue, '[OO]', 1, NULL) + self.assertRaises(SystemError, buildvalue, '[OO]', NULL, 2) + self.assertRaises(SystemError, buildvalue, '{OO}', 1, NULL) + self.assertRaises(SystemError, buildvalue, '{OO}', NULL, 2) + def test_buildvalue_N(self): _testcapi.test_buildvalue_N() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index ce3d0b1b1b005ca..c703360c0e042d3 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -386,6 +386,25 @@ raise_error(void *unused) return NULL; } +static PyObject * +py_buildvalue(PyObject *self, PyObject *args) +{ + const char *fmt; + PyObject *objs[10] = {NULL}; + if (!PyArg_ParseTuple(args, "s|OOOOOOOOOO", &fmt, + &objs[0], &objs[1], &objs[2], &objs[3], &objs[4], + &objs[5], &objs[6], &objs[7], &objs[8], &objs[9])) + { + return NULL; + } + for(int i = 0; i < 10; i++) { + NULLABLE(objs[i]); + } + return Py_BuildValue(fmt, + objs[0], objs[1], objs[2], objs[3], objs[4], + objs[5], objs[6], objs[7], objs[8], objs[9]); +} + static int test_buildvalue_N_error(const char *fmt) { @@ -3252,6 +3271,7 @@ static PyMethodDef TestMethods[] = { #endif {"getbuffer_with_null_view", getbuffer_with_null_view, METH_O}, {"PyBuffer_SizeFromFormat", test_PyBuffer_SizeFromFormat, METH_VARARGS}, + {"py_buildvalue", py_buildvalue, METH_VARARGS}, {"test_buildvalue_N", test_buildvalue_N, METH_NOARGS}, {"test_get_statictype_slots", test_get_statictype_slots, METH_NOARGS}, {"test_get_type_name", test_get_type_name, METH_NOARGS}, From e7f7e701a83a67c84f483d06f7cab4f978d3efc7 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 10 Oct 2023 11:46:38 +0300 Subject: [PATCH 2/4] Add also some tests for integer arguments --- Lib/test/test_capi/test_misc.py | 28 +++++++++++++++++++++++++++- Modules/_testcapimodule.c | 17 +++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index f5bb37fb85d3fd5..bc633e258e30e62 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -299,7 +299,7 @@ def test_getitem_with_error(self): self.assertIn('returned a result with an exception set', err) def test_buildvalue(self): - """Test Py_BuildValue()""" + """Test Py_BuildValue() with object arguments""" buildvalue = _testcapi.py_buildvalue self.assertEqual(buildvalue(''), None) self.assertEqual(buildvalue('()'), ()) @@ -352,6 +352,32 @@ def test_buildvalue(self): self.assertRaises(SystemError, buildvalue, '{OO}', 1, NULL) self.assertRaises(SystemError, buildvalue, '{OO}', NULL, 2) + def test_buildvalue_ints(self): + """Test Py_BuildValue() with integer arguments""" + buildvalue = _testcapi.py_buildvalue_ints + from _testcapi import SHRT_MIN, SHRT_MAX, USHRT_MAX, INT_MIN, INT_MAX, UINT_MAX + self.assertEqual(buildvalue('i', INT_MAX), INT_MAX) + self.assertEqual(buildvalue('i', INT_MIN), INT_MIN) + self.assertEqual(buildvalue('I', INT_MAX), INT_MAX) + + self.assertEqual(buildvalue('h', SHRT_MAX), SHRT_MAX) + self.assertEqual(buildvalue('h', SHRT_MIN), SHRT_MIN) + self.assertEqual(buildvalue('H', USHRT_MAX), USHRT_MAX) + + self.assertEqual(buildvalue('b', 127), 127) + self.assertEqual(buildvalue('b', -128), -128) + self.assertEqual(buildvalue('B', 255), 255) + + self.assertEqual(buildvalue('c', ord('A')), b'A') + self.assertEqual(buildvalue('c', 255), b'\xff') + self.assertEqual(buildvalue('c', 256), b'\x00') + self.assertEqual(buildvalue('c', -1), b'\xff') + + self.assertEqual(buildvalue('C', 255), chr(255)) + self.assertEqual(buildvalue('C', 256), chr(256)) + self.assertEqual(buildvalue('C', sys.maxunicode), chr(sys.maxunicode)) + self.assertRaises(ValueError, buildvalue, 'C', -1) + self.assertRaises(ValueError, buildvalue, 'C', sys.maxunicode+1) def test_buildvalue_N(self): _testcapi.test_buildvalue_N() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index c703360c0e042d3..2b9b2321c81e7be 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -405,6 +405,22 @@ py_buildvalue(PyObject *self, PyObject *args) objs[5], objs[6], objs[7], objs[8], objs[9]); } +static PyObject * +py_buildvalue_ints(PyObject *self, PyObject *args) +{ + const char *fmt; + unsigned int values[10] = {0}; + if (!PyArg_ParseTuple(args, "s|IIIIIIIIII", &fmt, + &values[0], &values[1], &values[2], &values[3], &values[4], + &values[5], &values[6], &values[7], &values[8], &values[9])) + { + return NULL; + } + return Py_BuildValue(fmt, + values[0], values[1], values[2], values[3], values[4], + values[5], values[6], values[7], values[8], values[9]); +} + static int test_buildvalue_N_error(const char *fmt) { @@ -3272,6 +3288,7 @@ static PyMethodDef TestMethods[] = { {"getbuffer_with_null_view", getbuffer_with_null_view, METH_O}, {"PyBuffer_SizeFromFormat", test_PyBuffer_SizeFromFormat, METH_VARARGS}, {"py_buildvalue", py_buildvalue, METH_VARARGS}, + {"py_buildvalue_ints", py_buildvalue_ints, METH_VARARGS}, {"test_buildvalue_N", test_buildvalue_N, METH_NOARGS}, {"test_get_statictype_slots", test_get_statictype_slots, METH_NOARGS}, {"test_get_type_name", test_get_type_name, METH_NOARGS}, From 7da5e9afd7e2eaf4948608665de14118b9269e51 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 10 Oct 2023 23:39:34 +0300 Subject: [PATCH 3/4] Turn docstrings into comments. --- Lib/test/test_capi/test_misc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index bc633e258e30e62..aa0e68bfc0d32e7 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -299,7 +299,7 @@ def test_getitem_with_error(self): self.assertIn('returned a result with an exception set', err) def test_buildvalue(self): - """Test Py_BuildValue() with object arguments""" + # Test Py_BuildValue() with object arguments buildvalue = _testcapi.py_buildvalue self.assertEqual(buildvalue(''), None) self.assertEqual(buildvalue('()'), ()) @@ -353,7 +353,7 @@ def test_buildvalue(self): self.assertRaises(SystemError, buildvalue, '{OO}', NULL, 2) def test_buildvalue_ints(self): - """Test Py_BuildValue() with integer arguments""" + # Test Py_BuildValue() with integer arguments buildvalue = _testcapi.py_buildvalue_ints from _testcapi import SHRT_MIN, SHRT_MAX, USHRT_MAX, INT_MIN, INT_MAX, UINT_MAX self.assertEqual(buildvalue('i', INT_MAX), INT_MAX) From 507be2a1df594e5ad6e94910a6e4018d69534866 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 11 Oct 2023 00:20:06 +0300 Subject: [PATCH 4/4] Update Lib/test/test_capi/test_misc.py Co-authored-by: Erlend E. Aasland --- Lib/test/test_capi/test_misc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index aa0e68bfc0d32e7..ef790c2a531bb9f 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -358,7 +358,7 @@ def test_buildvalue_ints(self): from _testcapi import SHRT_MIN, SHRT_MAX, USHRT_MAX, INT_MIN, INT_MAX, UINT_MAX self.assertEqual(buildvalue('i', INT_MAX), INT_MAX) self.assertEqual(buildvalue('i', INT_MIN), INT_MIN) - self.assertEqual(buildvalue('I', INT_MAX), INT_MAX) + self.assertEqual(buildvalue('I', UINT_MAX), UINT_MAX) self.assertEqual(buildvalue('h', SHRT_MAX), SHRT_MAX) self.assertEqual(buildvalue('h', SHRT_MIN), SHRT_MIN)