FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-34735: Fix a memory leak in Modules/timemodule.c by ZackerySpytz · Pull Request #9418 · python/cpython · GitHub

Repository navigation

bpo-34735: Fix a memory leak in Modules/timemodule.c - #9418

Merged
serhiy-storchaka merged 1 commit into
python:masterfrom
ZackerySpytz:bpo-34735-timemodule.c-leak
Sep 21, 2018
Merged

serhiy-storchaka merged 1 commit into
python:masterfrom
ZackerySpytz:bpo-34735-timemodule.c-leak

Conversation

ZackerySpytz commented Sep 19, 2018 •
edited by brettcannon
Loading

Copy link
Copy Markdown
Contributor

The format string was not being freed when ValueError was raised for %y on certain platforms.

There was a missing PyMem_Free(format) in time_strftime().

Copy link
Copy Markdown
Member

The actual fix here looks good to me, but I am fairly sure that this code is not hit by any of the tests, so I recommend adding a test that hits there.

I think adding this to Lib/test/datetimetester.py under TestDate should work:

@unittest.skipif(not (sys.platform.startswith('aix') or
                      sys.platform.startswith('win'))
                 "%y on Windows and AIX doesn't support years < 1900")
def test_strftime_aixwin_pre1900(self):
    d = self.theclass(1, 1, 1)

    self.assertRaises(ValueError, d.strftime, '%y')

Copy link
Copy Markdown
Contributor Author

@pganssle This code is hit in test_y_before_1900() in Lib/test/test_strftime.py.

pganssle commented Sep 19, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

@ZackerySpytz Ah, tricky tricky. There's a bunch of strftime-related tests in test_time and datetimetester so I didn't notice test_strftime.py. My bad.

Copy link
Copy Markdown
Member

Thanks for the patch, @ZackerySpytz ! I'm not sure if my label adding and removing messed up our auto-merge. If it did I will try to merge this tomorrow.

Copy link
Copy Markdown
Member

merge approved PR

serhiy-storchaka merged commit 91e6c87 into python:master Sep 21, 2018

Copy link
Copy Markdown
Contributor

Thanks @ZackerySpytz for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 21, 2018
There was a missing PyMem_Free(format) in time_strftime().
(cherry picked from commit 91e6c87)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>

Copy link
Copy Markdown

GH-9469 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 21, 2018
There was a missing PyMem_Free(format) in time_strftime().
(cherry picked from commit 91e6c87)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>

Copy link
Copy Markdown

GH-9470 is a backport of this pull request to the 3.6 branch.

miss-islington added a commit that referenced this pull request Sep 21, 2018
There was a missing PyMem_Free(format) in time_strftime().
(cherry picked from commit 91e6c87)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>
miss-islington added a commit that referenced this pull request Sep 21, 2018
There was a missing PyMem_Free(format) in time_strftime().
(cherry picked from commit 91e6c87)

Co-authored-by: Zackery Spytz <zspytz@gmail.com>

Copy link
Copy Markdown
Contributor Author

Thank you, @brettcannon and @serhiy-storchaka.

ZackerySpytz deleted the bpo-34735-timemodule.c-leak branch September 22, 2018 18:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL