Skip to content

gh-155742: Use PyBytesWriter in marshal - #155748

Open
vstinner wants to merge 3 commits into
python:mainfrom
vstinner:marshal
Open

gh-155742: Use PyBytesWriter in marshal#155748
vstinner wants to merge 3 commits into
python:mainfrom
vstinner:marshal

Conversation

@vstinner

@vstinner vstinner commented Aug 13, 2026

Copy link
Copy Markdown
Member

Replace soft deprecated PyBytes_FromStringAndSize() and _PyBytes_Resize() with PyBytesWriter.

Replace soft deprecated PyBytes_FromStringAndSize() and
_PyBytes_Resize() with PyBytesWriter.
No PyBytesWriter is needed.

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think the current code is broken.

Comment thread Python/marshal.c
Comment thread Python/marshal.c
@vstinner

Copy link
Copy Markdown
Member Author

I do not think the current code is broken.

I didn't say that the current code is broken. The PR only just avoids the soft deprecated PyBytes_FromStringAndSize() function.

I reworked the error handling.

@serhiy-storchaka: Please review the updated PR.

@vstinner

Copy link
Copy Markdown
Member Author

I wrote a script to test manually this PR by injecting MemoryError at different places:

import marshal
import io
import _testcapi
obj = b'x' * (1024 * 1024)
file = io.BytesIO()
for i in range(10):
    try:
        try:
            _testcapi.set_nomemory(i)
            res = marshal.dump(obj, file)
        finally:
            _testcapi.remove_mem_hooks()
    except Exception as exc:
        print(f"marshal.dump failed: {exc!r}")
    else:
        print(f"{res=}")

Before, the code failed with an assertion error. With my latest change, the code works is all cases (always raise MemoryError as expected).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review interpreter-core (Objects, Python, Grammar, and Parser dirs) skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants