Skip to content
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

Now allocates properly sized result buffer. #3657

Merged
merged 1 commit into from
Aug 1, 2016

Conversation

NovaDenizen
Copy link
Contributor

See #3649

@mention-bot
Copy link

@NovaDenizen, thanks for your PR! By analyzing the annotation information on this pull request, we identified @23Skidoo and @phadej to be potential reviewers

@ezyang
Copy link
Contributor

ezyang commented Aug 1, 2016

Thanks! I'll wait for Appveyor tests to go green and then I'll merge it.

(I checked to make sure we weren't committing an off-by-one-error. The docs are very confusing in this respect but I believe the code is right: querying for the needed buffer size will return enough space for the null byte, even though the success return does NOT include the null byte. Pshaw!)

@ezyang
Copy link
Contributor

ezyang commented Aug 1, 2016

@NovaDenizen I just realized that this code could have been made more robust in one slight way: we should check the return value of the second invocation and error if it is greater than the buffer size we passed in (this would have also caught the previous problem, instead of passing a half-constructed buffer back to the user.) Now that the code is right, I guess it's unnecessary though!

@ezyang ezyang merged commit 5a1f7d5 into haskell:master Aug 1, 2016
@23Skidoo
Copy link
Member

23Skidoo commented Aug 1, 2016

LGTM. Thanks, @NovaDenizen!

@23Skidoo
Copy link
Member

23Skidoo commented Aug 1, 2016

Also cherry-picked into 1.24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants