Skip to content
This repository was archived by the owner on Mar 11, 2025. It is now read-only.

Conversation

jackcmay
Copy link
Contributor

Token account definitions were not FFI compatible because of the use of Option<T>

  • use an explicit boolean instead
  • autogenerate C headers using cbindgen

@jackcmay
Copy link
Contributor Author

@garious @t-nelson fyi, using cbindgen to create C headers

@garious Option<T> is not FFI friendly so I replaced with an explicit boolean. Another option is to define our own "Option" that uses #[repr(C)], thoughts?

@garious
Copy link
Contributor

garious commented Jun 19, 2020

Our own Option sounds better to me. It's a fundamental building block these days.

Why add the auto-generated code to git?

@jackcmay
Copy link
Contributor Author

@garious I'll give our own options a shot, probably want that to live in our SDK eventually.

Checking in the header has two advantages, non-rust projects that want to pull use the header don't have to use Rust to generate it. Also its more explicit when someone intentionally or unintentionally makes a change that affects the public interface.

@jackcmay jackcmay force-pushed the ffi-compatible-interface branch from 99a35c4 to 8ae1535 Compare June 23, 2020 19:02
@jackcmay
Copy link
Contributor Author

@garious I added a repr(C) version of Option, was this the route you had in mind?

@jackcmay jackcmay force-pushed the ffi-compatible-interface branch 2 times, most recently from 58e8363 to 4d35ecc Compare June 24, 2020 22:12
@jackcmay jackcmay requested review from t-nelson and garious June 25, 2020 00:38
@jackcmay
Copy link
Contributor Author

Will move option.rs to the SDK once/if approved

@jackcmay jackcmay force-pushed the ffi-compatible-interface branch from 4d35ecc to 2c2c292 Compare June 26, 2020 16:11
@jackcmay
Copy link
Contributor Author

@garious What do you think of our use of own OptionReprC and putting it into the SDK?

@garious
Copy link
Contributor

garious commented Jun 26, 2020

@jackcmay, no objection. Buy maybe name it COption? Analogous to ffi's CString?

@jackcmay jackcmay merged commit f8f51c1 into solana-labs:master Jun 26, 2020
@jackcmay jackcmay deleted the ffi-compatible-interface branch June 26, 2020 22:20
andrewsource147 pushed a commit to MeteoraAg/solana-program-library that referenced this pull request May 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants