Skip to content

Remove use of locale-specific strerror_r. #440

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

Merged
merged 2 commits into from
May 31, 2024
Merged

Remove use of locale-specific strerror_r. #440

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8a69c5
Remove use of locale-specific `strerror_r`.
briansmith May 30, 2024
1cbce31
Use `std::io::Error` for `Debug` and `Display` when it is available.
briansmith May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 11 additions & 28 deletions src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#[cfg(feature = "std")]
extern crate std;

use core::{fmt, num::NonZeroU32};

/// A small and `no_std` compatible error type
Expand Down Expand Up @@ -98,35 +101,13 @@ impl Error {
}
}

cfg_if! {
if #[cfg(unix)] {
fn os_err(errno: i32, buf: &mut [u8]) -> Option<&str> {
let buf_ptr = buf.as_mut_ptr().cast::<libc::c_char>();
if unsafe { libc::strerror_r(errno, buf_ptr, buf.len()) } != 0 {
return None;
}

// Take up to trailing null byte
let n = buf.len();
let idx = buf.iter().position(|&b| b == 0).unwrap_or(n);
core::str::from_utf8(&buf[..idx]).ok()
Copy link
Contributor Author

@briansmith briansmith May 30, 2024

Choose a reason for hiding this comment

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

Even if the bytes do not contain any invalid UTF-8 sequences, they might not be encoding UTF-8.

Copy link
Member

@josephlr josephlr May 30, 2024

Choose a reason for hiding this comment

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

Is there an actual example of this being an issue on a unix target?

Copy link
Contributor Author

@briansmith briansmith May 30, 2024

Choose a reason for hiding this comment

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

No offense, but I don't participate in that kind of thinking. Instead it would be better to have a proof that it isn't an issue.

Copy link
Member

@josephlr josephlr May 30, 2024

Choose a reason for hiding this comment

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

Oh no offense taken, I think this is a good point (especially when it comes to stuff like the safety of code or cryptographic security where I 100% agree). Sorry for being a little short. I just meant to note that an implementation returning UTF8 bytes that aren't UTF8 only causes the display implementation to show a suboptimal error string.

However, after looking more into strerror_r, I agree that we shouldn't be assuming that it is well behaved on arbitrary unix targets. This is libstd's job, not ours. See my suggestion below.

}
} else {
fn os_err(_errno: i32, _buf: &mut [u8]) -> Option<&str> {
None
}
}
}

impl fmt::Debug for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut dbg = f.debug_struct("Error");
if let Some(errno) = self.raw_os_error() {
dbg.field("os_error", &errno);
let mut buf = [0u8; 128];
if let Some(err) = os_err(errno, &mut buf) {
dbg.field("description", &err);
}
#[cfg(feature = "std")]
dbg.field("description", &std::io::Error::from_raw_os_error(errno));
} else if let Some(desc) = internal_desc(*self) {
dbg.field("internal_code", &self.0.get());
dbg.field("description", &desc);
Expand All @@ -140,10 +121,12 @@ impl fmt::Debug for Error {
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if let Some(errno) = self.raw_os_error() {
let mut buf = [0u8; 128];
match os_err(errno, &mut buf) {
Some(err) => err.fmt(f),
None => write!(f, "OS Error: {}", errno),
cfg_if! {
if #[cfg(feature = "std")] {
std::io::Error::from_raw_os_error(errno).fmt(f)
} else {
write!(f, "OS Error: {}", errno)
}
}
} else if let Some(desc) = internal_desc(*self) {
f.write_str(desc)
Expand Down