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

Decrement reference count #7003

Merged
merged 6 commits into from Mar 22, 2023
Merged

Conversation

radarhere
Copy link
Member

Resolves #6323

Copy link
Contributor

@nulano nulano left a comment

Choose a reason for hiding this comment

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

Is there any reason you sometimes used Py_DECREF instead of Py_XDECREF?

It looks to me like it might be just for PyUnicode_FromString, but I don't see anything special about that function.

@radarhere
Copy link
Member Author

Just for the times when we know that the value isn't null. Looking at the documentation, I had concluded that PyUnicode_FromString doesn't ever return null, unlike PyLong_FromLong.

@nulano
Copy link
Contributor

nulano commented Mar 20, 2023

I am pretty sure that is an omission in the documentation:

https://github.com/python/cpython/blob/5e6661bce968173fa45b74fa2111098645ff609c/Objects/unicodeobject.c#L1836-L1845

I'd expect every function that can allocate memory to possibly fail and return NULL.

@radarhere
Copy link
Member Author

Ok, I've pushed an update.

@hugovk
Copy link
Member

hugovk commented Mar 21, 2023

@nulano How does this look now?

@nulano
Copy link
Contributor

nulano commented Mar 21, 2023

I've got one suggestion - I'm working on a PR to open in @radarhere's repo.

Edit: see radarhere#19

@radarhere
Copy link
Member Author

Just to be clear, part of @nulano's suggestion is to remove

>>> from PIL import _imagingmorph
>>> _imagingmorph.__version
'0.1'

It's an unused part of the C API, so no objections.

@hugovk hugovk merged commit e7fa309 into python-pillow:main Mar 22, 2023
65 checks passed
@radarhere radarhere deleted the reference_count branch March 22, 2023 07:51
Sign up for free to 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.

Memory leak bugs when a new reference is only passed to a non-stealing API (static analyzer reports)
3 participants