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

sdk: consider switching away from null terminated strings to ptr and length #646

Open
G4Vi opened this issue Dec 28, 2023 · 1 comment
Open
Assignees

Comments

@G4Vi
Copy link
Contributor

G4Vi commented Dec 28, 2023

Taking \0 terminated strings is convenient for C development using the str functions in libc, but is not optimal for other use cases. It causes extra work in implementation of languages specific sdks and requires extism to often recompute string lengths that are already known.

Working on the C++ SDK, I noticed several functions take const std::string & and then pass the c string with .c_str() to extism. Often the passed in strings will be static string literals, that in order to be passed as const std::string &, a std::string must be constructed, likely with a heap allocation. This issue can be worked around by providing duplicate functions in the C++ sdk that take const char * instead, however this clutters the C++ SDK. The modern C++ solution to pass readonly string data is std::string_view, which can be cheaply made from a std::string or a c string or even a substring, it just stores pointer and length. Unfortunately with the current extism sdk, it isn't helpful as a std::string_view is not necessarily null terminated, so to use it with extism the string would need to be unnecessarily copied.

If we switch extism to always take pointer and length for strings, I can eliminate the duplicate functions in the C++ SDK. This likely would also result in cleaner, faster, and less verbose code for the other SDKs based on top of the C SDK.

@G4Vi
Copy link
Contributor Author

G4Vi commented Jan 8, 2024

After discussion in week of the 1st, we decided to hold off on this for now. It's a breaking change that would complicate FFI usage (automatic conversion to and from native string types in various languages).

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