Compare error description without allocating

help
1

In Tokio we have a piece of code like this:

if inner.description() == MY_ERROR {
}

which checks if an std::io::Error is the one we created with a specific string using Error::new(ErrorKind::Other, MY_ERROR). However, this is broken on recent versions of the Rust language because description() now returns:

inner.description() = "description() is deprecated; use Display"

How can I update this comparison to avoid description() without allocating memory?

2 Likes
2

Maybe a custom wrapper type, instead of avoiding description?

#[derive(Debug)]
struct MyError(String);

impl std::fmt::Display for MyError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
        write!(f, "{}", self.0)
    }
}

impl std::error::Error for MyError {
    fn description(&self) -> &str {
        &self.0
    }
}
3

The error type is std::io::Error. This can't be changed.

4

So the inner error needs to be io::Error, too? I interpreted your code as MY_ERROR being a &'static str, so I thought we could construct the outer io::Error via io::Error::new(io::ErrorKind::Other, MyError(MY_ERROR)), getting to the description of the inner error via io::Error::get_ref.

1 Like
5

Would something like this work?

2 Likes
6

Here's another possibility for comparing a impl Display with a &str without allocating a String, though if you can skip the formatting machinery it will likely be better:

pub fn eq(d: impl std::fmt::Display, s: &str) -> bool {
    use std::fmt::Write;
    
    struct FormatEq<'r> {
        remainder: &'r str,
        unequal: bool,
    }
    
    impl<'r> Write for FormatEq<'r> {
        fn write_str(&mut self, s: &str) -> std::fmt::Result {
            if !self.unequal {
                if let Some(new_remainder) = self.remainder.strip_prefix(s) {
                    self.remainder = new_remainder;
                } else {
                    self.unequal = true;
                }
            }
            Ok(())
        }
    }
    
    let mut fmt_eq = FormatEq { remainder: s, unequal: false };
    _ = write!(fmt_eq, "{}", d);
    fmt_eq.remainder.is_empty() && !fmt_eq.unequal
}
4 Likes
7

The FormatEq idea is neat, thanks.

@SkiFire13 If you want to retain credit for the solution, then do you mind submitting a PR to Tokio?

diff --git a/tokio/src/process/unix/pidfd_reaper.rs b/tokio/src/process/unix/pidfd_reaper.rs
index c87eddf3..b522d123 100644
--- a/tokio/src/process/unix/pidfd_reaper.rs
+++ b/tokio/src/process/unix/pidfd_reaper.rs
@@ -95,14 +95,41 @@ where
     pidfd: PollEvented<Pidfd>,
 }
 
+fn display_eq(d: impl std::fmt::Display, s: &str) -> bool {
+    use std::fmt::Write;
+
+    struct FormatEq<'r> {
+        remainder: &'r str,
+        unequal: bool,
+    }
+
+    impl<'r> Write for FormatEq<'r> {
+        fn write_str(&mut self, s: &str) -> std::fmt::Result {
+            if !self.unequal {
+                if let Some(new_remainder) = self.remainder.strip_prefix(s) {
+                    self.remainder = new_remainder;
+                } else {
+                    self.unequal = true;
+                }
+            }
+            Ok(())
+        }
+    }
+
+    let mut fmt_eq = FormatEq {
+        remainder: s,
+        unequal: false,
+    };
+    let _ = write!(fmt_eq, "{}", d);
+    fmt_eq.remainder.is_empty() && !fmt_eq.unequal
+}
+
 #[allow(deprecated)]
 fn is_rt_shutdown_err(err: &io::Error) -> bool {
     if let Some(inner) = err.get_ref() {
-        // Using `Error::description()` is more efficient than `format!("{inner}")`,
-        // so we use it here even if it is deprecated.
         err.kind() == io::ErrorKind::Other
             && inner.source().is_none()
-            && inner.description() == RUNTIME_SHUTTING_DOWN_ERROR
+            && display_eq(inner, RUNTIME_SHUTTING_DOWN_ERROR)
     } else {
         false
     }
1 Like
8

Thanks but I'm a bit busy right now :sweat_smile: Just linking to this thread from the PR is fine for me.

That said I think @Schard's approach of using get_ref + downcast_ref is both cleaner and faster, though it requires creating a custom struct for RUNTIME_SHUTTING_DOWN_ERROR instead of using a plain string.

1 Like
9

I'd recommend replacing the strings then with an appropriate error enum.

10

Right now I'm just concerned with working around the breaking change in the stdlib. Using a custom enum requires changes in several places in the codebase, and while that may be a good idea, I don't think it should be part of a bugfix.

1 Like
11

I'd disagree. Introducing a complex hack instead of cleaning up the architecture when compensating for an API change would not be acceptable for me. But then, I am not the crate author, you are. :wink:

1 Like
12

Ok, but the other change is also complex. Is it a breaking change to introduce an enum like this? It could break downstream crates downcasting std::io::Error to string. I think it's probably acceptable, but I don't want to find out with the bugfix, especially not if I backport that fix to LTS, since if I end up needing to revert it, then the revert needs to be backported too.

3 Likes