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 CStr as a pointer rather than a slice #59905

Open
czipperz opened this issue Apr 12, 2019 · 15 comments
Open

Implement CStr as a pointer rather than a slice #59905

czipperz opened this issue Apr 12, 2019 · 15 comments
Labels
A-FFI Area: Foreign function interface (FFI) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@czipperz
Copy link
Contributor

czipperz commented Apr 12, 2019

Littered throughout the CStr documentation are notes and hints that it should be eventually converted to using a pointer underneath the hood instead of a slice. This change will make CStr::from_ptr O(1) instead of O(n). Other constructors may still be O(n) because they have to check to ensure there is only one null terminator. If someone wants to try implementing this, it seems like a pretty simple change.

Current struct definition:

pub struct CStr {
    inner: [c_char]
}

Proposed:

pub struct CStr {
    inner: *const c_char
}
@czipperz
Copy link
Contributor Author

I'm not sure if this needs an RFC. If so I can make it.

@czipperz
Copy link
Contributor Author

Potential complications: CStr must remain !Sized. Is there a way to explicitly mark a struct as not Sized?
Can it just impl !Sized for CStr {}

@czipperz
Copy link
Contributor Author

The current definition makes &CStr roughly equivalent to *const c_char, whereas the proposed new definition would have another layer of indirection. That is, we want CStr to be the actual bytes in the string. So really we shouldn't be storing a pointer, but rather an unsized slice.

@czipperz
Copy link
Contributor Author

czipperz commented Apr 12, 2019

Perhaps we do something like

pub struct CStr {
    inner: [c_char; 1],
    _marker: PhantomUnsized
}

But that would require PhantomUnsized to exist.

@jonas-schievink jonas-schievink added A-FFI Area: Foreign function interface (FFI) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 12, 2019
@Kampfkarren
Copy link
Contributor

Kampfkarren commented Apr 12, 2019

_marker: [PhantomData<()>]?

@LifeIsStrange
Copy link

LifeIsStrange commented Apr 13, 2019

This seems to be prior art: https://github.com/kennytm/thin_cstr
@kennytm

@czipperz
Copy link
Contributor Author

Thanks @LifeIsStrange . I couldn't find an open issue for this so I'll keep it open

@LifeIsStrange
Copy link

here are the blockers (custom DST support) kennytm/thin_cstr#1

@lambda-fairy
Copy link
Contributor

Is it worth adding a link in the documentation to this issue? So that future readers know where to look.

@RalfJung
Copy link
Member

RalfJung commented Apr 14, 2019

pub struct CStr {
    inner: [c_char]
}

Proposed:

pub struct CStr {
    inner: *const c_char
}

This seems wrong. CStr is defined with the intention that *mut CStr is like char* in C. Your proposed CStr has one pointer indirection too many.

Proper CStr is blocked on custom DSTs, AFAIK.

@czipperz
Copy link
Contributor Author

@RalfJung I already noted it above. It's a good point though and an interesting design decision

@tgross35
Copy link
Contributor

Does #81513 contain what is needed to make this work?

@bobogei81123
Copy link

Before this is resolved, is there a way to create a null pointer of *const CStr?
I tried std::ptr::null() and it failed because CStr does not implement Thin.

@tgross35
Copy link
Contributor

tgross35 commented Jun 7, 2023

@bobogei81123 CStr is a bit of a weird type - &CStr is actually two pointers wide, it stores both a pointer and a length (same for &str or &[T]).

What do you need the pointer for? If you need to interface with a C method that takes a string, use a null *const c_char. If you’re only interfacing with Rust, &CStr or Option<&CStr> should be able to do what you need.

@bobogei81123
Copy link

Oh got it. I thought *const CStr is just *const c_char with the additional contract that it is null terminated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) C-enhancement Category: An issue proposing an enhancement or a PR with one. 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

8 participants