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.

Sign up for GitHub

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

Jump to bottom

bpo-36367: Free buffer if realloc fails in tokenize.c #12442

Merged
merged 1 commit into from Mar 19, 2019
Merged

bpo-36367: Free buffer if realloc fails in tokenize.c #12442

merged 1 commit into from Mar 19, 2019

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Mar 19, 2019 •
edited by bedevere-bot

@pablogsal pablogsal force-pushed the bpo36367 branch 2 times, most recently from 9c036e5 to dce2421 Compare March 19, 2019 16:20
matrixise
matrixise approved these changes Mar 19, 2019
View reviewed changes
Copy link
Member

@matrixise matrixise left a comment

Choose a reason for hiding this comment

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

I had the same solution, seems to be fine for me.

remilapeyre
remilapeyre suggested changes Mar 19, 2019
View reviewed changes
Copy link
Contributor

@remilapeyre remilapeyre left a comment

Choose a reason for hiding this comment

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

Hi @pablogsal I think there is the same issue in https://github.com/python/cpython/blob/dce242187f25b6861b08fe5d7e581efcfc49ea21/Parser/tokenizer.c#L891. Can you fix it too?

I think the cast to char * there is also not needed.

@remilapeyre
Copy link
Contributor

As far as I can tell this issue is also present on https://github.com/python/cpython/blob/dce242187f25b6861b08fe5d7e581efcfc49ea21/Parser/tokenizer.c#L963

vstinner
vstinner reviewed Mar 19, 2019
View reviewed changes
Parser/tokenizer.c Outdated
@@ -618,7 +618,7 @@ static char *
translate_newlines(const char *s, int exec_input, struct tok_state *tok) {
int skip_next_lf = 0;
size_t needed_length = strlen(s) + 2, final_length;
char *buf, *current;
char *buf, *current, *result;
Copy link
Member

Choose a reason for hiding this comment

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

Can you move result declaration where it's used? Python 3.8 now allows C99 ;-) (see PEP 7)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! :)

@pablogsal
Copy link
Member Author

@remilapeyre I think for https://github.com/python/cpython/blob/dce242187f25b6861b08fe5d7e581efcfc49ea21/Parser/tokenizer.c#L891 is not a problem as is handle correctly afterwards:

https://github.com/python/cpython/blob/dce242187f25b6861b08fe5d7e581efcfc49ea21/Parser/tokenizer.c#L893-L899

remilapeyre
remilapeyre approved these changes Mar 19, 2019
View reviewed changes
Copy link
Contributor

@remilapeyre remilapeyre left a comment

Choose a reason for hiding this comment

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

@pablogsal Yes it is, I think you patch is correct now.

@pablogsal pablogsal merged commit cb90c89 into python:master Mar 19, 2019
@pablogsal pablogsal deleted the bpo36367 branch March 19, 2019 17:18
@miss-islington
Copy link
Contributor

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-12470 is a backport of this pull request to the 2.7 branch.

@bedevere-bot
Copy link

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

vstinner added a commit that referenced this pull request Mar 20, 2019
@vstinner
bpo-36367: Free buffer if realloc fails in tokenize.c (GH-12442) (GH-…
65b9849 
…12471)
vstinner added a commit that referenced this pull request Mar 20, 2019
@vstinner
bpo-36367: Free buffer if realloc fails in tokenize.c (GH-12442) (GH-…
469b0a5 
…12470)

(cherry picked from commit cb90c89)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner left review comments

@matrixise matrixise matrixise approved these changes

@remilapeyre remilapeyre remilapeyre approved these changes

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@pablogsal @remilapeyre @miss-islington @bedevere-bot @matrixise @vstinner @the-knights-who-say-ni