Socket: fix inconsistencies in argument types #4136
Merged
Conversation
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
|
@tannewt for background, I had a long chat with @hierophect on discord in #circuitpython about this. I think this PR does some good cleanup. Some general-purpose validation routines could be added, say in a top-level @hierophect looks like you need a (maybe) merge from upstream and definitely |
|
I even made a note to do the translate this time and I somehow forgot anyway |
|
Thanks for the quick turnaround! I built this branch and flashed it onto my FeatherS2 and it works just fine, at least for |
|
Fine by me! @dhalbert please review. |
dhalbert
approved these changes
Feb 8, 2021
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.
Suggestion cannot be applied right now. Please check back later.
The Common-Hal API for Socket had some inconsistencies argument size and type between functions, such as the "port" argument. Some of these were even using uint8_t, which is far too small for many common port values. This PR converts all arguments to uint32_t and adds some sign checking since Python always passes them in as signed ints.
Further down the line, it may be appropriate to add more comprehensive input sanitation to this and other shared-bindings implementations.
Resolves #4127