Skip to content

bpo-22831: Use "with" to avoid possible fd leaks in distutils.#10921

Merged
serhiy-storchaka merged 1 commit intopython:masterfrom
serhiy-storchaka:fd-leaks-distutils
Dec 20, 2018
Merged

bpo-22831: Use "with" to avoid possible fd leaks in distutils.#10921
serhiy-storchaka merged 1 commit intopython:masterfrom
serhiy-storchaka:fd-leaks-distutils

Conversation

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Dec 5, 2018

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

FYI I rebased the PR on master and used "git diff master.. -w" to hide indentation changes.

@vstinner
Copy link
Member

vstinner commented Dec 5, 2018

Oh, it's also possible to ignore whitespaces by adding ?w=1 to the URL!
https://github.com/python/cpython/pull/10921/files?w=1

@tirkarthi
Copy link
Member

I think this might help in having a review from @merwok given that there is similar open issue with changes to distutils and some comments on the other issue's PR changes https://bugs.python.org/issue35416#msg331114

@merwok
Copy link
Member

merwok commented Dec 20, 2018

This looks OK. Does it replace the other PR?

@serhiy-storchaka
Copy link
Member Author

It intersects with the other PR. It is an extraction from a larger patch which changed all code that calls close() explicitly. Seems the other PR contains also other changes.

@serhiy-storchaka serhiy-storchaka merged commit c5d5dfd into python:master Dec 20, 2018
@serhiy-storchaka serhiy-storchaka deleted the fd-leaks-distutils branch December 20, 2018 17:00
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.

6 participants