Review feedback

Author: zooba

Doc/c-api/long.rst

@@ -387,9 +387,12 @@ distinguished from a number.  Use :c:func:`PyErr_Occurred` to disambiguate.
    bytes were copied into the buffer.
 
    Passing *n_bytes* of zero will always return the requested buffer size.
-   The requested buffer size may be larger than necessary, but only when the
-   larger size is required to contain the full value. This function is not an
-   accurate way to determine the bit length of a value.
+
+   .. note::
+
+      When the value does not fit in the provided buffer, the requested size
+      returned from the function may be larger than necessary. Passing 0 to this
+      function is not an accurate way to determine the bit length of a value.
 
    .. versionadded:: 3.13
 

Doc/whatsnew/3.13.rst

@@ -1493,7 +1493,7 @@ New Features
 
 * Add :c:func:`PyLong_AsNativeBytes`, :c:func:`PyLong_FromNativeBytes` and
   :c:func:`PyLong_FromUnsignedNativeBytes` functions to simplify converting
-  between native integer types and Python ``int`` objects.
+  between native integer types and Python :class:`int` objects.
   (Contributed by Steve Dower in :gh:`111140`.)
 
 

Lib/test/test_capi/test_long.py

@@ -426,7 +426,11 @@ def test_long_asvoidptr(self):
 
     def test_long_asnativebytes(self):
         import math
-        from _testcapi import pylong_asnativebytes as asnativebytes, SIZE_MAX
+        from _testcapi import (
+            pylong_asnativebytes as asnativebytes,
+            INT_MAX,
+            SIZE_MAX,
+        )
 
         # Abbreviate sizeof(Py_ssize_t) to SZ because we use it a lot
         SZ = int(math.ceil(math.log(SIZE_MAX + 1) / math.log(2)) / 8)
@@ -453,6 +457,9 @@ def test_long_asnativebytes(self):
                 buffer = bytearray(1)
                 self.assertEqual(expect, asnativebytes(v, buffer, 0, -1),
                     "PyLong_AsNativeBytes(v, NULL, 0, -1)")
+                # Also check via the __index__ path
+                self.assertEqual(expect, asnativebytes(Index(v), buffer, 0, -1),
+                    "PyLong_AsNativeBytes(Index(v), NULL, 0, -1)")
 
         # We request as many bytes as `expect_be` contains, and always check
         # the result (both big and little endian). We check the return value
@@ -514,6 +521,19 @@ def test_long_asnativebytes(self):
                     f"PyLong_AsNativeBytes(v, buffer, {n}, <little>)")
                 self.assertEqual(expect_le, buffer[:n], "<little>")
 
+        # Check a few error conditions. These are validated in code, but are
+        # unspecified in docs, so if we make changes to the implementation, it's
+        # fine to just update these tests rather than preserve the behaviour.
+        with self.assertRaises(SystemError):
+            asnativebytes(1, buffer, 0, 2)
+        with self.assertRaises(TypeError):
+            asnativebytes('not a number', buffer, 0, -1)
+        with self.assertRaises(SystemError):
+            # n_bytes is taken as size_t and clamped to 'int'. With the sign
+            # change, we should always be able to pass INT_MAX+1 and see this
+            # failure.
+            asnativebytes(1, buffer, INT_MAX + 1, -1)
+
     def test_long_fromnativebytes(self):
         import math
         from _testcapi import (
@@ -546,5 +566,14 @@ def test_long_fromnativebytes(self):
                 self.assertEqual(expect_u, fromnativebytes(v_le, n, 1, 0),
                     f"PyLong_FromUnsignedNativeBytes(buffer, {n}, <little>)")
 
+                # Check native endian when the result would be the same either
+                # way and we can test it.
+                if v_be == v_le:
+                    self.assertEqual(expect_s, fromnativebytes(v_be, n, -1, 1),
+                        f"PyLong_FromNativeBytes(buffer, {n}, <native>)")
+                    self.assertEqual(expect_u, fromnativebytes(v_be, n, -1, 0),
+                        f"PyLong_FromUnsignedNativeBytes(buffer, {n}, <native>)")
+
+
 if __name__ == "__main__":
     unittest.main()

Modules/_testcapi/long.c

@@ -790,7 +790,9 @@ pylong_asnativebytes(PyObject *module, PyObject *args)
         PyBuffer_Release(&buffer);
         return NULL;
     }
-    if (buffer.len < n) {
+    // Allow n > INT_MAX for tests - it will error out without writing to the
+    // buffer, so no overrun issues.
+    if (buffer.len < n && n <= INT_MAX) {
         PyErr_SetString(PyExc_ValueError, "buffer must be at least 'n' bytes");
         PyBuffer_Release(&buffer);
         return NULL;

Objects/longobject.c

@@ -1080,6 +1080,19 @@ _fits_in_n_bits(Py_ssize_t v, Py_ssize_t n)
     return v_extended == 0 || v_extended == -1;
 }
 
+static inline int
+resolve_endianness(int *endianness)
+{
+    if (*endianness < 0) {
+        *endianness = PY_LITTLE_ENDIAN;
+    }
+    if (*endianness != 0 && *endianness != 1) {
+        PyErr_SetString(PyExc_SystemError, "invalid 'endianness' value");
+        return -1;
+    }
+    return 0;
+}
+
 int
 PyLong_AsNativeBytes(PyObject* vv, void* buffer, size_t n, int endianness)
 {
@@ -1102,11 +1115,7 @@ PyLong_AsNativeBytes(PyObject* vv, void* buffer, size_t n, int endianness)
     }
 
     int little_endian = endianness;
-    if (endianness < 0) {
-        endianness = PY_LITTLE_ENDIAN;
-    }
-    if (endianness != 0 && endianness != 1) {
-        PyErr_SetString(PyExc_SystemError, "invalid 'endianness' value");
+    if (resolve_endianness(&little_endian) < 0) {
         return -1;
     }
 
@@ -1226,11 +1235,7 @@ PyLong_FromNativeBytes(const void* buffer, size_t n, int endianness)
     }
 
     int little_endian = endianness;
-    if (endianness < 0) {
-        endianness = PY_LITTLE_ENDIAN;
-    }
-    if (endianness != 0 && endianness != 1) {
-        PyErr_SetString(PyExc_SystemError, "invalid 'endianness' value");
+    if (resolve_endianness(&little_endian) < 0) {
         return NULL;
     }
 
@@ -1248,11 +1253,7 @@ PyLong_FromUnsignedNativeBytes(const void* buffer, size_t n, int endianness)
     }
 
     int little_endian = endianness;
-    if (endianness < 0) {
-        endianness = PY_LITTLE_ENDIAN;
-    }
-    if (endianness != 0 && endianness != 1) {
-        PyErr_SetString(PyExc_SystemError, "invalid 'endianness' value");
+    if (resolve_endianness(&little_endian) < 0) {
         return NULL;
     }