-
-
Notifications
You must be signed in to change notification settings - Fork 33.4k
Closed
Labels
Description
There's a fast-path in PyBytes_FromStringAndSize that is not covered:
Lines 120 to 124 in 8549559
| if (size == 1 && str != NULL) { | |
| op = CHARACTER(*str & 255); | |
| assert(_Py_IsImmortal(op)); | |
| return (PyObject *)op; | |
| } |
Here:
Lines 111 to 124 in 8549559
| PyObject * | |
| PyBytes_FromStringAndSize(const char *str, Py_ssize_t size) | |
| { | |
| PyBytesObject *op; | |
| if (size < 0) { | |
| PyErr_SetString(PyExc_SystemError, | |
| "Negative size passed to PyBytes_FromStringAndSize"); | |
| return NULL; | |
| } | |
| if (size == 1 && str != NULL) { | |
| op = CHARACTER(*str & 255); | |
| assert(_Py_IsImmortal(op)); | |
| return (PyObject *)op; | |
| } |
We don't have test cases for it:
cpython/Lib/test/test_capi/test_bytes.py
Lines 49 to 65 in 8549559
| def test_fromstringandsize(self): | |
| # Test PyBytes_FromStringAndSize() | |
| fromstringandsize = _testlimitedcapi.bytes_fromstringandsize | |
| self.assertEqual(fromstringandsize(b'abc'), b'abc') | |
| self.assertEqual(fromstringandsize(b'abc', 2), b'ab') | |
| self.assertEqual(fromstringandsize(b'abc\0def'), b'abc\0def') | |
| self.assertEqual(fromstringandsize(b'', 0), b'') | |
| self.assertEqual(fromstringandsize(NULL, 0), b'') | |
| self.assertEqual(len(fromstringandsize(NULL, 3)), 3) | |
| self.assertRaises((MemoryError, OverflowError), | |
| fromstringandsize, NULL, PY_SSIZE_T_MAX) | |
| self.assertRaises(SystemError, fromstringandsize, b'abc', -1) | |
| self.assertRaises(SystemError, fromstringandsize, b'abc', PY_SSIZE_T_MIN) | |
| self.assertRaises(SystemError, fromstringandsize, NULL, -1) | |
| self.assertRaises(SystemError, fromstringandsize, NULL, PY_SSIZE_T_MIN) |
I propose adding:
self.assertEqual(fromstringandsize(b'a'), b'a')
self.assertEqual(fromstringandsize(b'a', 1), b'a')So we would have better C coverage stats. It won't hurt in any case, even if this path is removed some day.