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

Passing pointers to ZST to FFI #142

Closed
elichai opened this issue Aug 8, 2019 · 4 comments
Closed

Passing pointers to ZST to FFI #142

elichai opened this issue Aug 8, 2019 · 4 comments

Comments

@elichai
Copy link
Member

elichai commented Aug 8, 2019

We have a bunch of functions that accept slices, these slices could be Zero Sized types if they're empty.
getting a pointer to ZST is implementation defined and could be NULL which might violates the ffi contract for non null values.
We should either explicitly check that the slices aren't empty or have our own pointer trait that always return a valid pointer

https://doc.rust-lang.org/nomicon/exotic-sizes.html#zero-sized-types-zsts

Example of having "our own as_ptr" https://github.com/enigmampc/enigma-core/blob/develop/enigma-types/src/traits.rs

I personally lean towards the first option of explicitly checking, the only problem with that is that we might forget. then it will hopefully call secp256k1_callback_call in the good case, and in a bad case create a UB

@apoelstra
Copy link
Member

If we get UB that is a bug upstream. No such bugs should exist.

@elichai
Copy link
Member Author

elichai commented Aug 8, 2019

Well upstream docs do specify Cannot be NULL sometimes, I agree it should be checked there and I'm pretty sure it is checked everywhere, but the docs don't explicitly say that.

This was referenced Aug 8, 2019
@apoelstra
Copy link
Member

Every single cannot be NULL should be accompanied by an ARG_CHECK asserting that the argument is non-NULL. I'm not aware of any exceptions.

@elichai
Copy link
Member Author

elichai commented Aug 22, 2019

Solved by #151

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