gh-90716: Refactor PyLong_FromString to separate concerns #96808
+296
−256
Conversation
This file contains 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
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
|
Please create a new github feature request issue to track this work. |
gpshead
changed the title
gpshead
added
the
interpreter-core
oscarbenjamin
changed the title
I don't really understand this workflow but does #96812 work for this? |
gpshead
changed the title
|
yep, that makes sense, though we've already got an issue for that - edited/redirected to it. :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
This is a preliminary PR to refactor
PyLong_FromStringwhich is currently quite messy and has spaghetti like code that mixes up different concerns as well as duplicating logic.In particular:
PyLong_FromStringnow only handles sign, base and prefix detection and calls a new functionlong_from_string_baseto parse the main body of the string.long_from_string_basefunction handles all string validation and then callslong_from_binary_baseor a new functionlong_from_non_binary_baseto construct the actualPyLong.long_from_binary_basefunction is simplified by factoring duplicated logic tolong_from_string_base.long_from_non_binary_basefactors out much of the code fromPyLong_FromStringincluding in particular the quadratic algorithm reffered to in CVE-2020-10735: Prevent DoS by large int<->str conversions #95778 so that this can be seen separately from unrelated concerns such as string validation.I intend to follow up on this with a PR to improve the algorithm used for decimal and other non binary bases but I think that would be a lot easier to do after this refactoring. I could also submit that algorithm in the same PR but I thought it would be easier to review this refactoring separately from a change of algorithm.