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

Unify FFI calls with an example on C# #114

Draft
wants to merge 2 commits into
base: csharp/integ_yuryf_rust
Choose a base branch
from

Conversation

Yury-Fridlyand
Copy link

@Yury-Fridlyand Yury-Fridlyand commented Feb 23, 2024

The C signature of new FFI function is

void command(void * client, unsigned long callback_id, int request_type, unsigned long arg_count, char ** args);

Signed-off-by: Yury-Fridlyand <[email protected]>
@@ -94,6 +98,16 @@ private void FailureCallback(ulong index)

#region FFI function declarations

public enum RequestType : uint

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove that and share the RequestType in the header file?
I'm not sure we need to convert from RequestType to RequestType to Cmd when it's 1:1:1.

Copy link
Author

@Yury-Fridlyand Yury-Fridlyand Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible. We can define the enum into a C header file and then import it to C# project and to rust (using bindgen).
We can also use protoc to do this for us and suppress a warning about not-FFI-safe enum.

Signed-off-by: Yury-Fridlyand <[email protected]>
Copy link

@aaron-congo aaron-congo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with C# or Rust but don't see any problems with this. Will just have to clean up a bit if this becomes a non-draft (eg removing the commented out prints etc)

) -> Result<Vec<String>, Utf8Error> {
from_raw_parts(data, len)
.iter()
.map(|arg| CStr::from_ptr(*arg).to_str().map(ToString::to_string))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use CString

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

Successfully merging this pull request may close these issues.

3 participants