Skip to content

Conversation

@vstinner
Copy link
Member

@vstinnervstinner 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
MemberAuthor

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

Copy link
Member

@serhiy-storchakaserhiy-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.

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

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
MemberAuthor

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
MemberAuthor

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) 

@vstinnervstinner merged commit f1e751e into python:mainOct 20, 2023
@vstinnervstinner deleted the asutf8andsize branch October 20, 2023 18:03
@vstinner
Copy link
MemberAuthor

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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vstinner@serhiy-storchaka