-
-
Notifications
You must be signed in to change notification settings - Fork 32k
PyUnicode_FSConverter() has confusing reference semantics #90241
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
Comments
The PyUnicode_FSConverter function has confusing reference semantics, and confusing documentation. https://docs.python.org/3/c-api/unicode.html#c.PyUnicode_FSConverter says the output argument "must be a PyBytesObject* which must be released when it is no longer used." That seems to suggest one must pass a PyBytesObject to it, and indeed one of the error paths assumes an object was passed (https://github.com/python/cpython/blob/main/Objects/unicodeobject.c#L4116-- 'addr' is called 'result' in the docs). Not passing a valid object would result in trying to DECREF NULL, or garbage. However, the function doesn't actually use the object, and later in the function overwrites the value *without* DECREFing it, so passing a valid object would in fact cause a leak. I understand the function signature is the way it is so it can be used with PyArg_ParseTuple's O& format, but there are reasons to call it directly (e.g. with METH_O functions), and it would be nice if the semantics were more clear. |
…yUnicode_FSDecoder
Another gotcha, if you're used to “regular” C API functions, are the return values. Should we also document these? And perhaps explicitly point out that it's different than most C API functions, in case readers skim past the numbers? e.g.: On failure, return 0 with an exception set.
On success, return ``Py_CLEANUP_SUPPORTED`` is *obj* was set, or 1 if *obj* is ``NULL``.
(Note that the values are different from most C API functions.) |
…de_FSDecoder (GH-128451) Co-authored-by: Stan Ulbrych <[email protected]> Co-authored-by: Erlend E. Aasland <[email protected]>
…yUnicode_FSDecoder (pythonGH-128451) (cherry picked from commit 657d7b7) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Stan Ulbrych <[email protected]> Co-authored-by: Erlend E. Aasland <[email protected]>
…yUnicode_FSDecoder (pythonGH-128451) (cherry picked from commit 657d7b7) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Stan Ulbrych <[email protected]> Co-authored-by: Erlend E. Aasland <[email protected]>
…PyUnicode_FSDecoder (GH-128451) (GH-128542) (cherry picked from commit 657d7b7) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Stan Ulbrych <[email protected]> Co-authored-by: Erlend E. Aasland <[email protected]>
…PyUnicode_FSDecoder (GH-128451) (GH-128543) (cherry picked from commit 657d7b7) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Stan Ulbrych <[email protected]> Co-authored-by: Erlend E. Aasland <[email protected]>
…yUnicode_FSDecoder (pythonGH-128451) Co-authored-by: Stan Ulbrych <[email protected]> Co-authored-by: Erlend E. Aasland <[email protected]>
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: