-
Notifications
You must be signed in to change notification settings - Fork 681
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
Change gethostname to use a buffer of MaybeUninit values #1745
Change gethostname to use a buffer of MaybeUninit values #1745
Conversation
87b7bb3
to
b47686e
Compare
Certainly an improvement. But wouldn't it be even better if it just returned an owned string, like what |
I actually am using this for in a wrapper that returns an owned string. I wasn't sure if changing this to only work for owned strings would be an improvement, as its technically possible to take the user-created buffer and unsafely make it owned with the length data from the returned c-string. This method also allows users to use buffers they may already have. Making this wrapper work only for owned strings would be the most convenient for me, but it would break use cases where the user already has a buffer. I wasn't sure if such a change would be accepted, so I opted to make the simplest, least controversial change. I would be willing to change my PR so that it only works for owned buffers if that is deemed important. |
I think owned strings would be most convenient for everybody. It wouldn't be the most performant option, but how many people rely on maximum performance for |
Fair enough. I've updated the PR to return an |
CI failures seem to be due to |
@nathaniel-daniel it should be fixed if you rebase! |
7710fb6
to
22c4ba8
Compare
Rebased |
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.
bors r+
Changing
gethostname
to accept a buffer ofMaybeUninit
bytes allows the user to avoid needlessly initializing a buffer. This is a breaking API change.