Skip to content

Decrement reference count#7003

Merged
hugovk merged 6 commits intopython-pillow:mainfrom
radarhere:reference_count
Mar 22, 2023
Merged

Decrement reference count#7003
hugovk merged 6 commits intopython-pillow:mainfrom
radarhere:reference_count

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