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

Docs don't match type niche #1

Closed
programmerjake opened this issue May 31, 2021 · 4 comments
Closed

Docs don't match type niche #1

programmerjake opened this issue May 31, 2021 · 4 comments

Comments

@programmerjake
Copy link

Shouldn't the OptionFileHandle use *mut c_void and not NonNull, since null is a valid value?

https://github.com/sunfishcode/io-experiment/blob/41303e0015f8a751d3ec86c9105984a47f49d989/src/types.rs#L153-L166

@sunfishcode
Copy link
Owner

There are reasons to guess that CreateFile never returns NULL, even as a successful value. Unfortunately, I haven't found an authoritative statement on that anywhere yet. I'd be interested if anyone has more info on this.

It would be quite unfortunate here if NULL could be valid handle value, because that would mean there's no single value we can carve out as a niche for Option<OwnedHandle>.

@sunfishcode
Copy link
Owner

On further reflection, it seems best to change this to a RawHandle and just explicitly check for null in the TryFrom implementation. We still don't support null handles, but in the absence of a proper guarantee, at least now we'll panic if we ever get a null here, rather than executing UB. I've also now expanded the documentation to mention the expectation that the handle is non-null.

99c6332

@sunfishcode
Copy link
Owner

The documentation is now expanded to describe this, and the code is now changed to panic instead of doing UB if we ever get a null, so I think this issue is now fixed. Thanks for filing this!

@programmerjake
Copy link
Author

Thanks! Sorry I couldn't find any decent references for null-ness.

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

No branches or pull requests

2 participants