Skip to content

bpo-44396: Update multi-line-start location when reallocating tokenizer buffers#26676

Merged
miss-islington merged 1 commit intopython:mainfrom
pablogsal:bpo-44396
Jun 12, 2021
Merged

bpo-44396: Update multi-line-start location when reallocating tokenizer buffers#26676
miss-islington merged 1 commit intopython:mainfrom
pablogsal:bpo-44396

Conversation

@pablogsal
Copy link
Copy Markdown
Member

@pablogsal pablogsal commented Jun 11, 2021 •
edited by miss-islington
Loading

https://bugs.python.org/issue44396

Automerge-Triggered-By: GH:pablogsal

@pablogsal pablogsal requested a review from lysnikolaou as a code owner June 11, 2021 17:18
@pablogsal pablogsal added the needs backport to 3.10 only security fixes label Jun 11, 2021
@isidentical
Copy link
Copy Markdown
Member

isidentical commented Jun 11, 2021

Should we also check whether multiline_start is NULL in

cpython/Parser/tokenizer.c

Line 1885 in 4cb6ba1

if (c == EOF || (quote_size == 1 && c == '\n')) {

@pablogsal
Copy link
Copy Markdown
Member Author

pablogsal commented Jun 11, 2021

Should we also check whether multiline_start is NULL in

Hummm...why you think that is necessary? How can that happen?

@isidentical
Copy link
Copy Markdown
Member

isidentical commented Jun 11, 2021

If I am not missing anything, according to this (on your branch) when the tok->multi_line_start - tok->buf < 0, then multi_line_start becomes NULL.
https://github.com/python/cpython/blob/304ca6ddb0591405ca422b5442306b59b157e2ca/Parser/tokenizer.c#L388

isidentical
isidentical approved these changes Jun 12, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@isidentical isidentical left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jun 12, 2021

@pablogsal: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit a342cc5 into python:main Jun 12, 2021
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Jun 12, 2021

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

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jun 12, 2021
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Jun 12, 2021

GH-26695 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 12, 2021
@pablogsal @miss-islington
bpo-44396: Update multi-line-start location when reallocating tokeniz…
b09e865 
…er buffers (pythonGH-26676)

Automerge-Triggered-By: GH:pablogsal
(cherry picked from commit a342cc5)

Co-authored-by: Pablo Galindo <[email protected]>
@pablogsal pablogsal deleted the bpo-44396 branch June 12, 2021 17:54
pablogsal pushed a commit that referenced this pull request Jun 12, 2021
@miss-islington
bpo-44396: Update multi-line-start location when reallocating tokeniz…
d03f342 
…er buffers (GH-26676) (GH-26695)

Automerge-Triggered-By: GH:pablogsal
(cherry picked from commit a342cc5)
jdevries3133 pushed a commit to jdevries3133/cpython that referenced this pull request Jun 19, 2021
@pablogsal @jdevries3133
bpo-44396: Update multi-line-start location when reallocating tokeniz…
b1e8a39 
…er buffers (pythonGH-26676)

Automerge-Triggered-By: GH:pablogsal
@pablogsal pablogsal mentioned this pull request Jul 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@isidentical isidentical isidentical approved these changes

@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pablogsal @isidentical @miss-islington @bedevere-bot @the-knights-who-say-ni