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

ACP: CStr::is_empty() #106

Closed
jmillikin opened this issue Sep 17, 2022 · 3 comments
Closed

ACP: CStr::is_empty() #106

jmillikin opened this issue Sep 17, 2022 · 3 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@jmillikin
Copy link

jmillikin commented Sep 17, 2022

Proposal

Problem statement

The str and slice primitives both have is_empty() to check if a value is empty, but CStr currently doesn't. This is unfortunate because the CStr::to_bytes() function is documented as performing a length calculation in the future, which would make cstr_val.to_bytes().is_empty() into an O(N) operation.

Motivation, use-cases

There's many reasons a user might want to check if a C string is empty, for example when writing FFI bindings to code written in C that crashes if given an empty string.

Solution sketches

pub const fn is_empty(&self) -> bool {
    (unsafe { self.inner.get_unchecked(0) }) == &0
}

Links and related work

https://rust-for-linux.github.io/docs/kernel/str/struct.CStr.html#method.is_empty

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@jmillikin jmillikin added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Sep 17, 2022
@thomcc
Copy link
Member

thomcc commented Sep 17, 2022

While I'm not sure about the specific implementation, this API seems worth adding.

@jmillikin
Copy link
Author

Updated the solution sketch to be less sketch-y.

Proposed implementation: jmillikin/upstream__rust@29b261d

Docs screenshot:

cstr-is-empty

@m-ou-se
Copy link
Member

m-ou-se commented Oct 11, 2022

This briefly came up in the libs-api meetings. This looks fine to us. :)

@m-ou-se m-ou-se closed this as completed Oct 11, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 11, 2022
…mulacrum

Add `is_empty()` method to `core::ffi::CStr`.

ACP: rust-lang/libs-team#106

Tracking issue: rust-lang#102444
RalfJung pushed a commit to RalfJung/miri that referenced this issue Oct 12, 2022
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Feb 10, 2023
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 17, 2023
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jul 18, 2023
@dtolnay dtolnay added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Nov 23, 2023
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

4 participants