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

Implement a function that creates a Cow<'_, CStr> from a slice of bytes #71279

Open
NickKolpinskiy opened this issue Apr 18, 2020 · 4 comments
Open
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@NickKolpinskiy
Copy link

I've been working on FFI-bindings for a C library and I ran into an issue where CString::new() would fail if there's a 0-byte.

I was left with three options:

  1. Let the users of my crate deal with NulError, which makes it less ergonomic, IMO;
  2. Just unwrap the Result under the hood and document the possible panic;
  3. When CString::new() fails, call CStr::from_bytes_with_nul() using NulError::nul_position(), wrap both Ok and Err branches in Cow::from().

I found the third option to be the most ergonomic and that's what I went with. Of course you could argue that it's sort of a leaky abstraction because users are expected to understand that C-libraries usually read until the 0-byte. But an interior 0-byte in a Rust string is not really a common case, is it?

So, what do you guys think, is this something worthy of being a part of std::ffi?

I already made the changes locally and I'd be happy to make a PR. However, I can't come up with a good name, so far I called it CString::from_slice() but I don't think it's fitting.

@NickKolpinskiy
Copy link
Author

@rustbot modify labels: +T-libs, +C-feature-request

@rustbot rustbot added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 18, 2020
@Mark-Simulacrum
Copy link
Member

I'm uncertain that std would want to expose an API that dealt with 0 bytes by early truncating, it seems like a pretty odd desire to me (I would personally return an error, I think, and document that it's invalid input).

I do think that an API returning Result<Cow<CStr>, InteriorNulByte> could work, though. We'd return the CStr if there was a null byte at the very end, a CString if there was no null byte (and so we have to allocate to get a 0-terminated buffer, and an error on interior null bytes.

I do wonder whether that's good to have, though -- at least in Rust, I think most use cases I've dealt with don't want this sort of handling, as if they're providing a null byte that's not with the intent that it gets used to terminate the string.

@NickKolpinskiy
Copy link
Author

I would personally return an error, I think, and document that it's invalid input.

It felt a bit awkward to use Result<T, E> with the builder pattern. Users would need to call unwrap() either after using setters that accept strings or after the final call (which, right now, returns neither Option nor Result). Maybe this is something I shouldn't have worried about.

I do think that an API returning Result<Cow<CStr>, InteriorNulByte> could work, though.

I could work on this. But since you said "I do wonder whether that's good to have", do you want to wait for someone else's input on this first?

@Mark-Simulacrum
Copy link
Member

If you don't mind that work being plausibly wasted, then you can work on it and open a PR.

Otherwise someone from libs team will need to chime in, but that can take time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants