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.

Sign up for GitHub

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

Jump to bottom

Replace ZStr with CStr #336

Closed
sunfishcode opened this issue May 21, 2022 · 0 comments · Fixed by #337
Closed

Replace ZStr with CStr #336

sunfishcode opened this issue May 21, 2022 · 0 comments · Fixed by #337
Assignees
@sunfishcode
Labels
semver bump Issues that will require a semver-incompatible fix

Comments

@sunfishcode
Copy link
Member

Rust has moved CStr into libcore and CString into liballoc: rust-lang/rust#94079

As an aside, I'm still fond of the idea of using u8 instead of c_char for this, because there isn't any reason to care about the signedness of C's char in nul-terminated string contexts—if you want to interpret the contents, you need to know the encoding, and the signedness of char doesn't help with that. Exposing c_char at this level just means that more places are exposed to gratuitous platform differences. But it's also true that in practice this issue is just a minor nuisance. Rust already had CStr with c_char, and replacing it with something that uses u8 would create a lot of churn, so keeping it makes sense.

So now that CStr has moved to core in nightly, that eliminates the need for ZStr in rustix, so rustix should switch to using CStr now.
@sunfishcode sunfishcode added the semver bump Issues that will require a semver-incompatible fix label May 21, 2022
@sunfishcode sunfishcode self-assigned this May 21, 2022
sunfishcode added a commit that referenced this issue May 21, 2022
@sunfishcode
Remove ZStr and ZString.
af05ed2 
Use `CStr` and `CString` instead, now that nightly has then in core and
alloc, respectively.

Fixes #336.
@sunfishcode sunfishcode mentioned this issue May 21, 2022
Merged
sunfishcode added a commit that referenced this issue May 27, 2022
@sunfishcode
Remove ZStr and ZString.
630f88a 
Use `CStr` and `CString` instead, now that nightly has then in core and
alloc, respectively.

Fixes #336.
sunfishcode added a commit that referenced this issue May 27, 2022
@sunfishcode
Remove ZStr and ZString. (#337)
b437157 
* Remove `ZStr` and `ZString`.

Use `CStr` and `CString` instead, now that nightly has then in core and
alloc, respectively.

Fixes #336.

* Enable "core_c_str" on nightly to use core::ffi::CStr.

* Don't test --no-default-features on stable.

--no-default-features disables "std", and building without "std" will
require nightly now.

* `CString` and `NulError` are in alloc.

* Add comments to the relevant Cargo.toml features.

* rustfmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sunfishcode sunfishcode

Labels
semver bump Issues that will require a semver-incompatible fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove ZStr and ZString. bytecodealliance/rustix
1 participant
@sunfishcode