Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-111140: Improve PyLong_AsNativeBytes API doc example & improve the test#115380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
9 commits Select commit Hold shift + click to select a range
0302840 Fix the API name in the doc example, improve test.
gpshead 5099616 further test refinements.
gpshead 4859a9b Merge branch 'main' into fixup/gh-11140
gpshead 97292ba One more assertion against n=0 writes.
gpshead 71d527f Clarify, document both use cases.
gpshead b04aa97 A little more verbose to avoid PyErr_Occurred.
gpshead cf1dd9c Update Doc/c-api/long.rst
gpshead ed5b887 remove redundant text that bloats the note.
gpshead 937df33 Switch the sentence in the note.
gpshead File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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*:: | ||
| int value; | ||
| Py_ssize_t bytes = PyLong_AsNativeBytes(v, &value, sizeof(value), -1); | ||
| int32_t value; | ||
| Py_ssize_t bytes = PyLong_AsNativeBits(pylong, &value, sizeof(value), -1); | ||
| if (bytes < 0){ | ||
| // Error occurred | ||
| // A Python exception was set with the reason. | ||
| return NULL; | ||
| } | ||
| else if (bytes <= (Py_ssize_t)sizeof(value)){ | ||
| // Success! | ||
| } | ||
| 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 | ||
| 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 | ||
| 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 | ||
| with as much of the value as can fit. This allows the caller to ignore all | ||
| non-negative results if the intent is to match the typical behavior of a | ||
| C-style downcast. No exception is set for this case. | ||
| Unless an exception is raised, all *n_bytes* of the buffer will always be | ||
| written. In the case of truncation, as many of the lowest bits of the value | ||
| as could fit are written. This allows the caller to ignore all non-negative | ||
| 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 | ||
gpshead marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| 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 | ||
| zero sign bit, which is redundant when the intention is to treat the value as | ||
| unsigned. | ||
| zero sign bit -- redundant if the caller's intention is to treat the value | ||
| 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:: | ||
| 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. | ||
gpshead marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| Passing *n_bytes=0* to this function is not an accurate way to determine | ||
| the bit length of a value. | ||
| .. versionadded:: 3.13 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -438,7 +438,12 @@ def test_long_asnativebytes(self): | ||
| if support.verbose: | ||
| 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) | ||
gpshead marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| for v, expect in [ | ||
| (0, SZ), | ||
| (512, SZ), | ||
| @@ -453,12 +458,25 @@ def test_long_asnativebytes(self): | ||
| (-(2**256-1), 33), | ||
| ]: | ||
| with self.subTest(f"sizeof-{v:X}"): | ||
| buffer = bytearray(1) | ||
| buffer = bytearray(b"\x5a") | ||
| 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 | ||
| 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 | ||
| # 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"): | ||
| 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] | ||
| self.assertEqual(expect_n, asnativebytes(v, buffer, n, 0), | ||
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.