Skip to content
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

gh-111089: PyUnicode_AsUTF8AndSize() sets size on error #111106

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 20, 2023

On error, PyUnicode_AsUTF8AndSize() now sets the size argument to 0, to avoid undefined value.


📚 Documentation preview 📚: https://cpython-previews--111106.org.readthedocs.build/

@vstinner
Copy link
Member Author

See also PR #111100: Add tests for failing PyUnicode_AsUTF8AndSize() with psize=NULL.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. See also #110865.

@@ -971,8 +971,8 @@ These are the UTF-8 codec APIs:
returned buffer always has an extra null byte appended (not included in
*size*), regardless of whether there are any other null code points.

In the case of an error, ``NULL`` is returned with an exception set and no
*size* is stored.
On error, set an exception, set *size* to 0 (if it's not NULL) and return
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
On error, set an exception, set *size* to 0 (if it's not NULL) and return
On error, set an exception, set *size* to ``0`` (if it's not ``NULL``) and return

@serhiy-storchaka
Copy link
Member

Why set size to 0, and not to -1? There is larger chance to miss error. For example PyUnicode_FromStringAndSize(NULL, 0) and PyBytes_FromStringAndSize(NULL, 0) return empty string and empty bytes object, but if size is negative, they raise exception.

On error, PyUnicode_AsUTF8AndSize() now sets the size argument to -1,
to avoid undefined value.
@vstinner
Copy link
Member Author

Why set size to 0, and not to -1?

That's a very good idea! I updated my PR to set size of -1 on error.

I just picked 0 randomly. I forgot that size can be negative.

@vstinner
Copy link
Member Author

Oh, tests failed with:

Testing of doctests in the sources finished, look at the results in build/doctest/output.txt.
python: ./Include/internal/pycore_typeobject.h:105: _PyType_GetModuleState: Assertion `et->ht_module' failed.
Aborted (core dumped)

@vstinner vstinner merged commit f1e751e into python:main Oct 20, 2023
@vstinner vstinner deleted the asutf8andsize branch October 20, 2023 18:03
@vstinner
Copy link
Member Author

I failed to reproduce the Sphinx crash locally. And the test passed when run again.

I merged my PR, thanks for review Serhiy.

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…#111106)

On error, PyUnicode_AsUTF8AndSize() now sets the size argument to -1,
to avoid undefined value.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…#111106)

On error, PyUnicode_AsUTF8AndSize() now sets the size argument to -1,
to avoid undefined value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants