gh-111140: Improve PyLong_AsNativeBytes API doc example & improve the test (#115380)

This expands the examples to cover both realistic use cases for the API.
    
I noticed thing in the test that could be done better so I added those as well: We need to guarantee that all bytes of the result are overwritten and that too many are not written.  Tests now pre-fills the result with data in order to ensure that.

Co-authored-by: Steve Dower <steve.dower@microsoft.com>
This commit is contained in:
Gregory P. Smith 2024-02-21 19:27:16 -08:00 committed by GitHub
parent 7f5e3f04f8
commit fac99b8b0d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 82 additions and 22 deletions

View file

@ -358,46 +358,86 @@ distinguished from a number. Use :c:func:`PyErr_Occurred` to disambiguate.
Copy the Python integer value to a native *buffer* of size *n_bytes*:: Copy the Python integer value to a native *buffer* of size *n_bytes*::
int value; int32_t value;
Py_ssize_t bytes = PyLong_AsNativeBytes(v, &value, sizeof(value), -1); Py_ssize_t bytes = PyLong_AsNativeBits(pylong, &value, sizeof(value), -1);
if (bytes < 0) { if (bytes < 0) {
// Error occurred // A Python exception was set with the reason.
return NULL; return NULL;
} }
else if (bytes <= (Py_ssize_t)sizeof(value)) { else if (bytes <= (Py_ssize_t)sizeof(value)) {
// Success! // Success!
} }
else { else {
// Overflow occurred, but 'value' contains truncated value // Overflow occurred, but 'value' contains the truncated
// lowest bits of pylong.
} }
The above example may look *similar* to
:c:func:`PyLong_As* <PyLong_AsSize_t>`
but instead fills in a specific caller defined type and never raises an
error about of the :class:`int` *pylong*'s value regardless of *n_bytes*
or the returned byte count.
To get at the entire potentially big Python value, this can be used to
reserve enough space and copy it::
// Ask how much space we need.
Py_ssize_t expected = PyLong_AsNativeBits(pylong, NULL, 0, -1);
if (expected < 0) {
// A Python exception was set with the reason.
return NULL;
}
assert(expected != 0); // Impossible per the API definition.
uint8_t *bignum = malloc(expected);
if (!bignum) {
PyErr_SetString(PyExc_MemoryError, "bignum malloc failed.");
return NULL;
}
// Safely get the entire value.
Py_ssize_t bytes = PyLong_AsNativeBits(pylong, bignum, expected, -1);
if (bytes < 0) { // Exception set.
free(bignum);
return NULL;
}
else if (bytes > expected) { // Be safe, should not be possible.
PyErr_SetString(PyExc_RuntimeError,
"Unexpected bignum truncation after a size check.");
free(bignum);
return NULL;
}
// The expected success given the above pre-check.
// ... use bignum ...
free(bignum);
*endianness* may be passed ``-1`` for the native endian that CPython was *endianness* may be passed ``-1`` for the native endian that CPython was
compiled with, or ``0`` for big endian and ``1`` for little. compiled with, or ``0`` for big endian and ``1`` for little.
Return ``-1`` with an exception raised if *pylong* cannot be interpreted as Returns ``-1`` with an exception raised if *pylong* cannot be interpreted as
an integer. Otherwise, return the size of the buffer required to store the an integer. Otherwise, return the size of the buffer required to store the
value. If this is equal to or less than *n_bytes*, the entire value was value. If this is equal to or less than *n_bytes*, the entire value was
copied. copied. ``0`` will never be returned.
Unless an exception is raised, all *n_bytes* of the buffer will be written Unless an exception is raised, all *n_bytes* of the buffer will always be
with as much of the value as can fit. This allows the caller to ignore all written. In the case of truncation, as many of the lowest bits of the value
non-negative results if the intent is to match the typical behavior of a as could fit are written. This allows the caller to ignore all non-negative
C-style downcast. No exception is set for this case. results if the intent is to match the typical behavior of a C-style
downcast. No exception is set on truncation.
Values are always copied as two's-complement, and sufficient buffer will be Values are always copied as two's-complement and sufficient buffer will be
requested to include a sign bit. For example, this may cause an value that requested to include a sign bit. For example, this may cause an value that
fits into 8 bytes when treated as unsigned to request 9 bytes, even though fits into 8 bytes when treated as unsigned to request 9 bytes, even though
all eight bytes were copied into the buffer. What has been omitted is the all eight bytes were copied into the buffer. What has been omitted is the
zero sign bit, which is redundant when the intention is to treat the value as zero sign bit -- redundant if the caller's intention is to treat the value
unsigned. as unsigned.
Passing zero to *n_bytes* will return the requested buffer size. Passing zero to *n_bytes* will return the size of a buffer that would
be large enough to hold the value. This may be larger than technically
necessary, but not unreasonably so.
.. note:: .. note::
When the value does not fit in the provided buffer, the requested size Passing *n_bytes=0* to this function is not an accurate way to determine
returned from the function may be larger than necessary. Passing 0 to this the bit length of a value.
function is not an accurate way to determine the bit length of a value.
.. versionadded:: 3.13 .. versionadded:: 3.13

View file

@ -438,7 +438,12 @@ def test_long_asnativebytes(self):
if support.verbose: if support.verbose:
print(f"SIZEOF_SIZE={SZ}\n{MAX_SSIZE=:016X}\n{MAX_USIZE=:016X}") print(f"SIZEOF_SIZE={SZ}\n{MAX_SSIZE=:016X}\n{MAX_USIZE=:016X}")
# These tests check that the requested buffer size is correct # These tests check that the requested buffer size is correct.
# This matches our current implementation: We only specify that the
# return value is a size *sufficient* to hold the result when queried
# using n_bytes=0. If our implementation changes, feel free to update
# the expectations here -- or loosen them to be range checks.
# (i.e. 0 *could* be stored in 1 byte and 512 in 2)
for v, expect in [ for v, expect in [
(0, SZ), (0, SZ),
(512, SZ), (512, SZ),
@ -453,12 +458,25 @@ def test_long_asnativebytes(self):
(-(2**256-1), 33), (-(2**256-1), 33),
]: ]:
with self.subTest(f"sizeof-{v:X}"): with self.subTest(f"sizeof-{v:X}"):
buffer = bytearray(1) buffer = bytearray(b"\x5a")
self.assertEqual(expect, asnativebytes(v, buffer, 0, -1), self.assertEqual(expect, asnativebytes(v, buffer, 0, -1),
"PyLong_AsNativeBytes(v, NULL, 0, -1)") "PyLong_AsNativeBytes(v, <unknown>, 0, -1)")
self.assertEqual(buffer, b"\x5a",
"buffer overwritten when it should not have been")
# Also check via the __index__ path # Also check via the __index__ path
self.assertEqual(expect, asnativebytes(Index(v), buffer, 0, -1), self.assertEqual(expect, asnativebytes(Index(v), buffer, 0, -1),
"PyLong_AsNativeBytes(Index(v), NULL, 0, -1)") "PyLong_AsNativeBytes(Index(v), <unknown>, 0, -1)")
self.assertEqual(buffer, b"\x5a",
"buffer overwritten when it should not have been")
# Test that we populate n=2 bytes but do not overwrite more.
buffer = bytearray(b"\x99"*3)
self.assertEqual(2, asnativebytes(4, buffer, 2, 0), # BE
"PyLong_AsNativeBytes(v, <3 byte buffer>, 2, 0) // BE")
self.assertEqual(buffer, b"\x00\x04\x99")
self.assertEqual(2, asnativebytes(4, buffer, 2, 1), # LE
"PyLong_AsNativeBytes(v, <3 byte buffer>, 2, 1) // LE")
self.assertEqual(buffer, b"\x04\x00\x99")
# We request as many bytes as `expect_be` contains, and always check # We request as many bytes as `expect_be` contains, and always check
# the result (both big and little endian). We check the return value # the result (both big and little endian). We check the return value
@ -510,7 +528,9 @@ def test_long_asnativebytes(self):
]: ]:
with self.subTest(f"{v:X}-{len(expect_be)}bytes"): with self.subTest(f"{v:X}-{len(expect_be)}bytes"):
n = len(expect_be) n = len(expect_be)
buffer = bytearray(n) # Fill the buffer with dummy data to ensure all bytes
# are overwritten.
buffer = bytearray(b"\xa5"*n)
expect_le = expect_be[::-1] expect_le = expect_be[::-1]
self.assertEqual(expect_n, asnativebytes(v, buffer, n, 0), self.assertEqual(expect_n, asnativebytes(v, buffer, n, 0),