Skip to content

Conversation

PixelDust22
Copy link
Contributor

@PixelDust22 PixelDust22 commented Jun 28, 2025

#134143 converted FromBytesWithNulError into an enum to address rust-lang/libs-team#493 (CStr::from_bytes_with_nul returns non-actionable error result)

However, the FromVecWithNulError returned from CString::from_vec_with_nul has a similar issue of returning non-actionable error result because you can't get the FromBytesWithNulErrorKind out of FromVecWithNulError.

Additionally, #134143 creates FromBytesWithNulError alongside the existing FromBytesWithNulErrorKind which is exactly the same.

This PR removes FromBytesWithNulErrorKind so that FromVecWithNulError uses FromBytesWithNulError directly. It also adds a new method kind to FromVecWithNulError so that the user can access the underlying error.

This is not a breaking change because FromBytesWithNulErrorKind is private, and we're only adding a method to FromVecWithNulError.

cc @nyurik

@rustbot
Copy link
Collaborator

rustbot commented Jun 28, 2025

r? @thomcc

rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 28, 2025
@workingjubilee
Copy link
Member

r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 28, 2025
@rustbot rustbot assigned dtolnay and unassigned thomcc Jun 28, 2025
@workingjubilee workingjubilee added the needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. label Jun 28, 2025
@rust-log-analyzer

This comment has been minimized.

@workingjubilee
Copy link
Member

@PixelDust22 You are proposing making an instantly stable change, which is generally not what we do even for small features if we can avoid it.

@PixelDust22
Copy link
Contributor Author

PixelDust22 commented Jun 28, 2025

@workingjubilee Given that #134143 was instantly stable, I don't see why this one couldn't be. This change really should've been a part of #134143.

@workingjubilee
Copy link
Member

@PixelDust22 Adding the getter function is a new API.

@workingjubilee
Copy link
Member

workingjubilee commented Jun 29, 2025

The change in #134143 was instantly stable because there was no way to feature flag replacing a struct with an enum. Adding a trivial function, however, is something that can be done under a feature flag. Usually doing so is just a formality, but it needs an FCP to add it as stable API in any case.


/// Access the underlying conversion error that was the cause of this error.
#[must_use]
#[stable(feature = "cstring_from_vec_with_nul", since = "1.58.0")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open a new library tracking issue for this method and change this attribute to unstable. The team always prefers adding new APIs unstably when possible.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 5, 2025
@Dylan-DPC
Copy link
Member

@PixelDust22 any updates on this? thanks

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

Labels

needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants