Cleanup socket APIs for other Rust consumers#36
Merged
Conversation
We should probably do a pass on this at some point, but consumers of
the Rust API should not have to deal with a boxed error generally so
that we avoid allocations and retain information about errors. We
would still have boxed errors in a few cases, most likely:
1. The errors on FFI boundaries will need boxed for ABI stability.
2. Having multiple errors from different crates in the same function.
Although it's possible to make error types to serve this case, it
does take effort that's probably better spent elsewhere for now.
Anyway, in this case, there is only one error type and the boxing is
unnecesary for Rust callers.
morrisonlevi
changed the title
ivoanjo
added a commit
that referenced
this pull request
Jun 18, 2025
* Add helper for creating CharSlice from a string literal
It looks like everyone that uses libddprof has a variant of this
macro, so I think it makes sense to just have it in `ffi.h`.
This specific implementation is built to only work with string
literals, aka there's no accidental/unexpected `strlen` going on.
* Improve error message when using ddprof_ffi_CharSlice_from_literal
This macro relies on a weird trick to fail compilation when you pass
in a char* and not a literal (the trick is that
`char *foo = "" "foo";` is valid but `char *bar; char *foo = "" bar;`
is not), but the error message would be kinda cryptic.
Since most compilers show the macro when the compilation fails,
I've added a nice comment which gets shown by compilers:
```
foo.c: In function 'endpoint_from':
foo.c:133:27: error: expected '}' before 's'
byte_slice_from_literal(s);
^
foo.c:15:113: note: in definition of macro 'byte_slice_from_literal'
/* NOTE: Compilation fails if you pass in a char* instead of a literal */ ((ddprof_ffi_CharSlice) {.ptr = "" string, .len = sizeof(string) - 1})
^~~~~~
foo.c:15:91: note: to match this '{'
/* NOTE: Compilation fails if you pass in a char* instead of a literal */ ((ddprof_ffi_CharSlice) {.ptr = "" string, .len = sizeof(string) - 1})
```
* Change name to DDPROF_FFI_CHARSLICE_C
Co-authored-by: Levi Morrison <levi.morrison@datadoghq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description and Motivation
We should probably do a pass on this at some point, but consumers of
the Rust API should not have to deal with a boxed error generally so
that we avoid allocations and retain information about errors. We
would still have boxed errors in a few cases, most likely:
Although it's possible to make error types to serve this case, it
does take effort that's probably better spent elsewhere for now.
Anyway, in this case, there is only one error type and the boxing is
unnecessary for Rust callers.
Additional Notes
I also changed some usages of
std::error::Errorto justErrorbecauseit's already imported.
I also exported
socket_path_from_uriinexporterto matchsocket_path_to_uri.How to test the change?
Most things will Just Work, but you may need to wrap the error in
Ok(...?)to get a conversion to another error type.