Self review

Author: vstinner

Doc/c-api/sys.rst

@@ -152,28 +152,29 @@ Operating System Utilities
       <c-preinit>` and so that the LC_CTYPE locale is properly configured: see
       the :c:func:`Py_PreInitialize` function.
 
-   Decode a byte string from the :term:`filesystem encoding and error handler`.
-   If the error handler is :ref:`surrogateescape error handler
-   <surrogateescape>`, undecodable bytes are decoded as characters in range
-   U+DC80..U+DCFF; and if a byte sequence can be decoded as a surrogate
-   character, the bytes are escaped using the surrogateescape error handler
-   instead of decoding them.
+   Decode a byte string from the :term:`filesystem encoding <filesystem
+   encoding and error handler>` with the :ref:`surrogateescape error handler
+   <surrogateescape>` error handler.
+
+   Undecodable bytes are decoded as characters in range U+DC80..U+DCFF. If a
+   byte sequence can be decoded as a surrogate character, escape the bytes
+   using the surrogateescape error handler instead of decoding them.
 
    Return a pointer to a newly allocated wide character string, use
    :c:func:`PyMem_RawFree` to free the memory. If size is not ``NULL``, write
    the number of wide characters excluding the null character into ``*size``
 
-   Return ``NULL`` on decoding error or memory allocation error. If *size* is
-   not ``NULL``, ``*size`` is set to ``(size_t)-1`` on memory error or set to
-   ``(size_t)-2`` on decoding error.
+   On memory allocation failure, set *\*size* to ``(size_t)-1`` and return
+   ``NULL``.
+
+   On decode error, set *\*size* to ``(size_t)-2`` and return ``NULL``.
+   Decoding errors should never happen, unless there is a bug in the C
+   library.
 
    The :term:`filesystem encoding and error handler` are selected by
    :c:func:`PyConfig_Read`: see :c:member:`~PyConfig.filesystem_encoding` and
    :c:member:`~PyConfig.filesystem_errors` members of :c:type:`PyConfig`.
 
-   Decoding errors should never happen, unless there is a bug in the C
-   library.
-
    Use the :c:func:`Py_EncodeLocale` function to encode the character string
    back to a byte string.
 
@@ -195,17 +196,19 @@ Operating System Utilities
 
 .. c:function:: char* Py_EncodeLocale(const wchar_t *text, size_t *error_pos)
 
-   Encode a wide character string to the :term:`filesystem encoding and error
-   handler`. If the error handler is :ref:`surrogateescape error handler
-   <surrogateescape>`, surrogate characters in the range U+DC80..U+DCFF are
-   converted to bytes 0x80..0xFF.
+   Encode a wide character string to the :term:`filesystem encoding <filesystem
+   encoding and error handler>` with the :ref:`surrogateescape error handler
+   <surrogateescape>`. Surrogate characters in the range U+DC80..U+DCFF are
+   encoded to bytes 0x80..0xFF.
 
    Return a pointer to a newly allocated byte string, use :c:func:`PyMem_Free`
-   to free the memory. Return ``NULL`` on encoding error or memory allocation
-   error.
+   to free the memory.
+
+   On memory allocation failure, set *\*error_pos* to ``(size_t)-1`` and return
+   ``NULL``.
 
-   If error_pos is not ``NULL``, ``*error_pos`` is set to ``(size_t)-1`` on
-   success,  or set to the index of the invalid character on encoding error.
+   On encoding error, set *\*error_pos* to the index of the first invalid
+   character and return ``NULL``.
 
    The :term:`filesystem encoding and error handler` are selected by
    :c:func:`PyConfig_Read`: see :c:member:`~PyConfig.filesystem_encoding` and

Lib/test/test_codecs.py

@@ -4072,11 +4072,12 @@ class LocaleCodecTest(unittest.TestCase):
     SURROGATES = "\uDC80\uDCFF"
 
     def encode_locale_surrogateescape(self, text):
-        # Test Py_EncodeLocale(): use the "surrogateescape" error handler
+        # Test public Py_EncodeLocale() C API:
+        # use the "surrogateescape" error handler
         return _testlimitedcapi.encode_locale(text)
 
-    def encode_locale_ex(self, text, errors="strict"):
-        # Test _Py_EncodeLocale()
+    def encode_locale(self, text, errors="strict"):
+        # Test the internal _Py_EncodeLocale() C API
         return _testinternalcapi.encode_locale(text, 0, errors)
 
     def check_encode_strings(self, errors):
@@ -4094,21 +4095,21 @@ def check_encode_strings(self, errors):
                         self.fail("failed to compute error_pos")
 
                     if errors == "surrogateescape":
-                        with self.assertRaises(ValueError) as cm:
+                        with self.assertRaises(RuntimeError) as cm:
                             self.encode_locale_surrogateescape(text)
-                        errmsg = f"Py_EncodeLocale failed: error_pos={error_pos}"
+                        errmsg = f"encode error: pos={error_pos}"
                         self.assertEqual(str(cm.exception), errmsg)
 
                     with self.assertRaises(RuntimeError) as cm:
-                        self.encode_locale_ex(text, errors)
+                        self.encode_locale(text, errors)
                     errmsg = f"encode error: pos={error_pos}"
                     self.assertEqual(str(cm.exception), errmsg)
                 else:
                     if errors in ("strict", "surrogateescape"):
                         encoded = self.encode_locale_surrogateescape(text)
                         self.assertEqual(encoded, expected)
 
-                    encoded = self.encode_locale_ex(text, errors)
+                    encoded = self.encode_locale(text, errors)
                     self.assertEqual(encoded, expected)
 
     def test_encode_strict(self):
@@ -4119,7 +4120,7 @@ def test_encode_surrogateescape(self):
 
     def test_encode_surrogatepass(self):
         try:
-            self.encode_locale_ex('', 'surrogatepass')
+            self.encode_locale('', 'surrogatepass')
         except ValueError as exc:
             if str(exc) == 'unsupported error handler':
                 self.skipTest(f"{self.ENCODING!r} encoder doesn't support "
@@ -4131,15 +4132,16 @@ def test_encode_surrogatepass(self):
 
     def test_encode_unsupported_error_handler(self):
         with self.assertRaises(ValueError) as cm:
-            self.encode_locale_ex('', 'backslashreplace')
+            self.encode_locale('', 'backslashreplace')
         self.assertEqual(str(cm.exception), 'unsupported error handler')
 
     def decode_locale(self, encoded, errors="strict"):
-        # Test _Py_DecodeLocale()
+        # Test the internal _Py_DecodeLocale() C API
         return _testinternalcapi.decode_locale(encoded, 0, errors)
 
     def decode_locale_surrogateescape(self, encoded):
-        # Test Py_DecodeLocale(): use the "surrogateescape" error handler
+        # Test the public Py_DecodeLocale() C API:
+        # use the "surrogateescape" error handler
         return _testlimitedcapi.decode_locale(encoded)
 
     def check_decode_strings(self, errors):
@@ -4179,7 +4181,7 @@ def check_decode_strings(self, errors):
                         self.fail("failed to compute error_pos")
 
                     if errors == "surrogateescape":
-                        with self.assertRaises(ValueError) as cm:
+                        with self.assertRaises(RuntimeError) as cm:
                             self.decode_locale_surrogateescape(encoded)
                         errmsg = f"Py_DecodeLocale failed: error_pos={error_pos}"
                         self.assertEqual(str(cm.exception), errmsg)

Modules/_testinternalcapi.c

@@ -1103,10 +1103,7 @@ encode_locale(PyObject *self, PyObject *args)
         PyErr_SetString(PyExc_ValueError, "unsupported error handler");
         break;
     default:
-        assert(str == NULL);
-        assert(output_length == 0);
-        assert(error_pos == 0);
-        PyErr_SetString(PyExc_ValueError, "unknown error code");
+        PyErr_SetString(PyExc_SystemError, "unknown error code");
         break;
     }
     return res;
@@ -1157,7 +1154,7 @@ decode_locale(PyObject *self, PyObject *args)
         PyErr_SetString(PyExc_ValueError, "unsupported error handler");
         break;
     default:
-        PyErr_SetString(PyExc_ValueError, "unknown error code");
+        PyErr_SetString(PyExc_SystemError, "unknown error code");
         break;
     }
     return res;

Modules/_testlimitedcapi/codec.c

@@ -25,25 +25,28 @@ decode_locale(PyObject *Py_UNUSED(module), PyObject *arg)
         return NULL;
     }
 
-    size_t wstr_len = (size_t)-123;
-    wchar_t *wstr = Py_DecodeLocale(str, &wstr_len);
+    const size_t size_canary = (size_t)-123;
+    size_t size = size_canary;
+    wchar_t *wstr = Py_DecodeLocale(str, &size);
 
     if (str == NULL) {
-        if (wstr_len == (size_t)-1) {
+        if (size == (size_t)-1) {
             PyErr_NoMemory();
         }
-        else if (wstr_len == (size_t)-2) {
-            PyErr_SetString(PyExc_ValueError, "decode error");
+        else if (size == (size_t)-2) {
+            PyErr_SetString(PyExc_RuntimeError, "decode error");
         }
         else {
             PyErr_Format(PyExc_SystemError,
                          "unknown Py_DecodeLocale() return value: %zd",
-                         (Py_ssize_t)wstr_len);
+                         (Py_ssize_t)size);
         }
         return NULL;
     }
+    assert(wstr != NULL);
+    assert(size != size_canary);
 
-    PyObject *result = PyUnicode_FromWideChar(wstr, wstr_len);
+    PyObject *result = PyUnicode_FromWideChar(wstr, size);
     PyMem_RawFree(wstr);
     return result;
 }
@@ -69,10 +72,14 @@ encode_locale(PyObject *Py_UNUSED(module), PyObject *arg)
     PyMem_Free(wstr);
 
     if (str == NULL) {
-        assert(error_pos != error_pos_canary);
-        return PyErr_Format(PyExc_ValueError,
-                            "Py_EncodeLocale failed: error_pos=%zd",
-                            error_pos);
+        if (error_pos == (size_t)-1) {
+            return PyErr_NoMemory();
+        }
+        else {
+            assert(error_pos != error_pos_canary);
+            return PyErr_Format(PyExc_RuntimeError,
+                                "encode error: pos=%zd", error_pos);
+        }
     }
     assert(error_pos == error_pos_canary);
 

Objects/unicodeobject.c

@@ -3773,12 +3773,12 @@ unicode_encode_locale(PyObject *unicode, _Py_error_handler error_handler,
     if (res != 0) {
         if (res == _Py_CODEC_ENCODE_ERROR) {
             PyObject *exc;
-            assert(error_pos <= (PY_SSIZE_T_MAX - 1));
+            assert(error_pos <= (size_t)(PY_SSIZE_T_MAX - 1));
             exc = PyObject_CallFunction(PyExc_UnicodeEncodeError, "sOnns",
                     "locale", unicode,
                     (Py_ssize_t)error_pos,
                     (Py_ssize_t)(error_pos+1),
-                    "encoding error");
+                    "encode error");
             if (exc != NULL) {
                 PyCodec_StrictErrors(exc);
                 Py_DECREF(exc);
@@ -3989,12 +3989,12 @@ unicode_decode_locale(const char *str, Py_ssize_t len,
     if (res != 0) {
         if (res == _Py_CODEC_DECODE_ERROR) {
             PyObject *exc;
-            assert(wlen <= (PY_SSIZE_T_MAX - 1));
+            assert(wlen <= (size_t)(PY_SSIZE_T_MAX - 1));
             exc = PyObject_CallFunction(PyExc_UnicodeDecodeError, "sy#nns",
                                         "locale", str, len,
                                         (Py_ssize_t)wlen,
                                         (Py_ssize_t)(wlen + 1),
-                                        "decoding error");
+                                        "decode error");
             if (exc != NULL) {
                 PyCodec_StrictErrors(exc);
                 Py_DECREF(exc);
@@ -5495,24 +5495,20 @@ PyUnicode_DecodeUTF8Stateful(const char *s,
 //
 // On memory allocation failure, return _Py_CODEC_MEMORY_ERROR.
 //
-// On decoding error (if surrogateescape is zero), return
-// _Py_CODEC_DECODE_ERROR. If wlen is non-NULL, write the start of the illegal
-// byte sequence into *wlen.
+// On decoding error (if errors is "strict"), return _Py_CODEC_DECODE_ERROR.
+// If wlen is non-NULL, write the start of the illegal byte sequence into
+// *wlen.
 //
-// Return _Py_CODEC_UNSUPPORTED_ERROR_HANDLER if 'errors' error handler is not
+// Return _Py_CODEC_UNSUPPORTED_ERROR_HANDLER if errors error handler is not
 // supported.
 int
 _Py_DecodeUTF8(const char *s, Py_ssize_t size, wchar_t **wstr, size_t *wlen,
                _Py_error_handler errors)
 {
+    assert(0 <= size);
     assert(s != NULL);
     assert(wstr != NULL);
 
-    const char *orig_s = s;
-    const char *e;
-    wchar_t *unicode;
-    Py_ssize_t outpos;
-
     int surrogateescape = 0;
     int surrogatepass = 0;
     switch (errors)
@@ -5531,18 +5527,18 @@ _Py_DecodeUTF8(const char *s, Py_ssize_t size, wchar_t **wstr, size_t *wlen,
 
     /* Note: size will always be longer than the resulting Unicode
        character count */
-    if (PY_SSIZE_T_MAX / (Py_ssize_t)sizeof(wchar_t) - 1 < size) {
+    if ((size_t)PY_SSIZE_T_MAX / sizeof(wchar_t) - 1 < (size_t)size) {
         return _Py_CODEC_MEMORY_ERROR;
     }
-
-    unicode = PyMem_RawMalloc((size + 1) * sizeof(wchar_t));
+    wchar_t *unicode = PyMem_RawMalloc((size + 1) * sizeof(wchar_t));
     if (!unicode) {
         return _Py_CODEC_MEMORY_ERROR;
     }
 
     /* Unpack UTF-8 encoded data */
-    e = s + size;
-    outpos = 0;
+    const char *orig_s = s;
+    const char *e = s + size;
+    Py_ssize_t outpos = 0;
     while (s < e) {
         Py_UCS4 ch;
 #if SIZEOF_WCHAR_T == 4