-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Add htonl, htons, ntohl, ntohs #3669
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
a3a9105
to
53b30d1
Compare
c6f1626
to
df4b69d
Compare
// It seems htonl, etc are macros on macOS. So we have to reimplement them. So let's | ||
// reimplement them for all UNIX platforms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could split the declaration for that target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could split the declaration for that target
@JohnTitor , yes, we can. But I think this is slightly more error-prone than my approach. I. e. if we split the declaration, then it is possible we introduce bug for some platform, but not for others
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally agree with JohnTitor here that most things should just use the host functions if available, concerns about bugs on one platform but not others isn't much of a concern when one of those platforms uses the system functions.
However, these are also straightforward enough that it doesn't seem worth the effort to split. So I think this is okay.
(backport <rust-lang#3669>) (cherry picked from commit 1feb354)
(backport <rust-lang#3669>) (cherry picked from commit 1feb354)
(backport <rust-lang#3669>) (cherry picked from commit 1feb354)
(backport <rust-lang#3669>) (cherry picked from commit 1feb354)
Not a breaking change