Merge pull request #505 from rust-embedded/write-ok-0

Clarify write() may only return Ok(0) if data is empty

Author: eldruin

embedded-io-adapters/CHANGELOG.md

@@ -8,7 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## Unreleased
 
 - Add support for adapting `BufRead` from `futures` and `tokio`.
+- Return an error when a wrapped `std`/`futures`/`tokio` `write()` call returns
+  `Ok(0)` to comply with `embedded_io::Write` requirements.
 
 ## 0.5.0 - 2023-08-06
 
-- First release
+- First release

embedded-io-adapters/src/futures_03.rs

@@ -57,7 +57,11 @@ impl<T: futures::io::AsyncBufRead + Unpin + ?Sized> embedded_io_async::BufRead f
 
 impl<T: futures::io::AsyncWrite + Unpin + ?Sized> embedded_io_async::Write for FromFutures<T> {
     async fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
-        poll_fn(|cx| Pin::new(&mut self.inner).poll_write(cx, buf)).await
+        match poll_fn(|cx| Pin::new(&mut self.inner).poll_write(cx, buf)).await {
+            Ok(0) if !buf.is_empty() => Err(std::io::ErrorKind::WriteZero.into()),
+            Ok(n) => Ok(n),
+            Err(e) => Err(e),
+        }
     }
 
     async fn flush(&mut self) -> Result<(), Self::Error> {

embedded-io-adapters/src/std.rs

@@ -1,5 +1,7 @@
 //! Adapters to/from `std::io` traits.
 
+use embedded_io::Error;
+
 /// Adapter from `std::io` traits.
 #[derive(Clone)]
 pub struct FromStd<T: ?Sized> {
@@ -52,7 +54,11 @@ impl<T: std::io::BufRead + ?Sized> embedded_io::BufRead for FromStd<T> {
 
 impl<T: std::io::Write + ?Sized> embedded_io::Write for FromStd<T> {
     fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
-        self.inner.write(buf)
+        match self.inner.write(buf) {
+            Ok(0) if !buf.is_empty() => Err(std::io::ErrorKind::WriteZero.into()),
+            Ok(n) => Ok(n),
+            Err(e) => Err(e),
+        }
     }
     fn flush(&mut self) -> Result<(), Self::Error> {
         self.inner.flush()
@@ -103,7 +109,11 @@ impl<T: embedded_io::Read + ?Sized> std::io::Read for ToStd<T> {
 
 impl<T: embedded_io::Write + ?Sized> std::io::Write for ToStd<T> {
     fn write(&mut self, buf: &[u8]) -> Result<usize, std::io::Error> {
-        self.inner.write(buf).map_err(to_std_error)
+        match self.inner.write(buf) {
+            Ok(n) => Ok(n),
+            Err(e) if e.kind() == embedded_io::ErrorKind::WriteZero => Ok(0),
+            Err(e) => Err(to_std_error(e)),
+        }
     }
     fn flush(&mut self) -> Result<(), std::io::Error> {
         self.inner.flush().map_err(to_std_error)

embedded-io-adapters/src/tokio_1.rs

@@ -68,7 +68,11 @@ impl<T: tokio::io::AsyncBufRead + Unpin + ?Sized> embedded_io_async::BufRead for
 
 impl<T: tokio::io::AsyncWrite + Unpin + ?Sized> embedded_io_async::Write for FromTokio<T> {
     async fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
-        poll_fn(|cx| Pin::new(&mut self.inner).poll_write(cx, buf)).await
+        match poll_fn(|cx| Pin::new(&mut self.inner).poll_write(cx, buf)).await {
+            Ok(0) if !buf.is_empty() => Err(std::io::ErrorKind::WriteZero.into()),
+            Ok(n) => Ok(n),
+            Err(e) => Err(e),
+        }
     }
 
     async fn flush(&mut self) -> Result<(), Self::Error> {

embedded-io-async/src/lib.rs

@@ -10,7 +10,7 @@ extern crate alloc;
 mod impls;
 
 pub use embedded_io::{
-    Error, ErrorKind, ErrorType, ReadExactError, ReadReady, SeekFrom, WriteAllError, WriteReady,
+    Error, ErrorKind, ErrorType, ReadExactError, ReadReady, SeekFrom, WriteReady,
 };
 
 /// Async reader.
@@ -125,13 +125,13 @@ pub trait Write: ErrorType {
     ///
     /// This function is not side-effect-free on cancel (AKA "cancel-safe"), i.e. if you cancel (drop) a returned
     /// future that hasn't completed yet, some bytes might have already been written.
-    async fn write_all(&mut self, buf: &[u8]) -> Result<(), WriteAllError<Self::Error>> {
+    async fn write_all(&mut self, buf: &[u8]) -> Result<(), Self::Error> {
         let mut buf = buf;
         while !buf.is_empty() {
             match self.write(buf).await {
-                Ok(0) => return Err(WriteAllError::WriteZero),
+                Ok(0) => panic!("write() returned Ok(0)"),
                 Ok(n) => buf = &buf[n..],
-                Err(e) => return Err(WriteAllError::Other(e)),
+                Err(e) => return Err(e),
             }
         }
         Ok(())

embedded-io/CHANGELOG.md

@@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+- Prohibit `Write::write` implementations returning `Ok(0)` unless there is no
+  data to write; consequently remove `WriteAllError` and the `WriteAllError`
+  variant of `WriteFmtError`. Update the `&mut [u8]` impl to possibly return
+  a new `SliceWriteError` if the slice is full instead of `Ok(0)`.
+- Add `WriteZero` variant to `ErrorKind` for implementations that previously
+  may have returned `Ok(0)` to indicate no further data could be written.
+- `Write::write_all` now panics if the `write()` implementation returns `Ok(0)`.
+
 ## 0.5.0 - 2023-08-06
 
 - Add `ReadReady`, `WriteReady` traits. They allow peeking whether the I/O handle is ready to read/write, so they allow using the traits in a non-blocking way.
@@ -37,4 +47,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## 0.2.0 - 2022-05-07
 
-- First release
+- First release

embedded-io/src/impls/slice_mut.rs

@@ -1,23 +1,56 @@
-use crate::{ErrorType, Write};
+use crate::{Error, ErrorKind, ErrorType, Write};
 use core::mem;
 
+// needed to prevent defmt macros from breaking, since they emit code that does `defmt::blahblah`.
+#[cfg(feature = "defmt-03")]
+use defmt_03 as defmt;
+
+/// Errors that could be returned by `Write` on `&mut [u8]`.
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[cfg_attr(feature = "defmt-03", derive(defmt::Format))]
+#[non_exhaustive]
+pub enum SliceWriteError {
+    /// The target slice was full and so could not receive any new data.
+    Full,
+}
+
+impl Error for SliceWriteError {
+    fn kind(&self) -> ErrorKind {
+        match self {
+            SliceWriteError::Full => ErrorKind::WriteZero,
+        }
+    }
+}
+
 impl ErrorType for &mut [u8] {
-    type Error = core::convert::Infallible;
+    type Error = SliceWriteError;
 }
 
+impl core::fmt::Display for SliceWriteError {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        write!(f, "{self:?}")
+    }
+}
+
+#[cfg(feature = "std")]
+#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
+impl std::error::Error for SliceWriteError {}
+
 /// Write is implemented for `&mut [u8]` by copying into the slice, overwriting
 /// its data.
 ///
 /// Note that writing updates the slice to point to the yet unwritten part.
 /// The slice will be empty when it has been completely overwritten.
 ///
 /// If the number of bytes to be written exceeds the size of the slice, write operations will
-/// return short writes: ultimately, `Ok(0)`; in this situation, `write_all` returns an error of
-/// kind `ErrorKind::WriteZero`.
+/// return short writes: ultimately, a `SliceWriteError::Full`.
 impl Write for &mut [u8] {
     #[inline]
     fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
         let amt = core::cmp::min(buf.len(), self.len());
+        if !buf.is_empty() && amt == 0 {
+            return Err(SliceWriteError::Full);
+        }
         let (a, b) = mem::take(self).split_at_mut(amt);
         a.copy_from_slice(&buf[..amt]);
         *self = b;

embedded-io/src/lib.rs

@@ -61,7 +61,6 @@ impl From<std::io::SeekFrom> for SeekFrom {
 /// This is the `embedded-io` equivalent of [`std::io::ErrorKind`], except with the following changes:
 ///
 /// - `WouldBlock` is removed, since `embedded-io` traits are always blocking. See the [crate-level documentation](crate) for details.
-/// - `WriteZero` is removed, since it is a separate variant in [`WriteAllError`] and [`WriteFmtError`].
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 #[cfg_attr(feature = "defmt-03", derive(defmt::Format))]
 #[non_exhaustive]
@@ -117,6 +116,8 @@ pub enum ErrorKind {
     /// An operation could not be completed, because it failed
     /// to allocate enough memory.
     OutOfMemory,
+    /// An attempted write could not write any data.
+    WriteZero,
 }
 
 #[cfg(feature = "std")]
@@ -248,19 +249,6 @@ impl From<ReadExactError<std::io::Error>> for std::io::Error {
     }
 }
 
-#[cfg(feature = "std")]
-#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl From<WriteAllError<std::io::Error>> for std::io::Error {
-    fn from(err: WriteAllError<std::io::Error>) -> Self {
-        match err {
-            WriteAllError::WriteZero => {
-                std::io::Error::new(std::io::ErrorKind::WriteZero, "WriteZero".to_owned())
-            }
-            WriteAllError::Other(e) => std::io::Error::new(e.kind(), format!("{e:?}")),
-        }
-    }
-}
-
 impl<E: fmt::Debug> fmt::Display for ReadExactError<E> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{self:?}")
@@ -275,8 +263,6 @@ impl<E: fmt::Debug> std::error::Error for ReadExactError<E> {}
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 #[cfg_attr(feature = "defmt-03", derive(defmt::Format))]
 pub enum WriteFmtError<E> {
-    /// [`Write::write`] wrote zero bytes
-    WriteZero,
     /// An error was encountered while formatting.
     FmtError,
     /// Error returned by the inner Write.
@@ -299,32 +285,6 @@ impl<E: fmt::Debug> fmt::Display for WriteFmtError<E> {
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
 impl<E: fmt::Debug> std::error::Error for WriteFmtError<E> {}
 
-/// Error returned by [`Write::write_all`]
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-#[cfg_attr(feature = "defmt-03", derive(defmt::Format))]
-pub enum WriteAllError<E> {
-    /// [`Write::write`] wrote zero bytes
-    WriteZero,
-    /// Error returned by the inner Write.
-    Other(E),
-}
-
-impl<E> From<E> for WriteAllError<E> {
-    fn from(err: E) -> Self {
-        Self::Other(err)
-    }
-}
-
-impl<E: fmt::Debug> fmt::Display for WriteAllError<E> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{self:?}")
-    }
-}
-
-#[cfg(feature = "std")]
-#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl<E: fmt::Debug> std::error::Error for WriteAllError<E> {}
-
 /// Blocking reader.
 ///
 /// This trait is the `embedded-io` equivalent of [`std::io::Read`].
@@ -401,11 +361,12 @@ pub trait Write: ErrorType {
     /// implementation to write an amount of bytes less than `buf.len()` while the writer continues to be
     /// ready to accept more bytes immediately.
     ///
-    /// Implementations should never return `Ok(0)` when `buf.len() != 0`. Situations where the writer is not
-    /// able to accept more bytes and likely never will are better indicated with errors.
+    /// Implementations must not return `Ok(0)` unless `buf` is empty. Situations where the
+    /// writer is not able to accept more bytes must instead be indicated with an error,
+    /// where the `ErrorKind` is `WriteZero`.
     ///
-    /// If `buf.len() == 0`, `write` returns without blocking, with either `Ok(0)` or an error.
-    /// The `Ok(0)` doesn't indicate an error.
+    /// If `buf` is empty, `write` returns without blocking, with either `Ok(0)` or an error.
+    /// `Ok(0)` doesn't indicate an error.
     fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error>;
 
     /// Flush this output stream, blocking until all intermediately buffered contents reach their destination.
@@ -419,12 +380,14 @@ pub trait Write: ErrorType {
     /// If you are using [`WriteReady`] to avoid blocking, you should not use this function.
     /// `WriteReady::write_ready()` returning true only guarantees the first call to `write()` will
     /// not block, so this function may still block in subsequent calls.
-    fn write_all(&mut self, mut buf: &[u8]) -> Result<(), WriteAllError<Self::Error>> {
+    ///
+    /// This function will panic if `write()` returns `Ok(0)`.
+    fn write_all(&mut self, mut buf: &[u8]) -> Result<(), Self::Error> {
         while !buf.is_empty() {
             match self.write(buf) {
-                Ok(0) => return Err(WriteAllError::WriteZero),
+                Ok(0) => panic!("write() returned Ok(0)"),
                 Ok(n) => buf = &buf[n..],
-                Err(e) => return Err(WriteAllError::Other(e)),
+                Err(e) => return Err(e),
             }
         }
         Ok(())
@@ -443,7 +406,7 @@ pub trait Write: ErrorType {
         // off I/O errors. instead of discarding them
         struct Adapter<'a, T: Write + ?Sized + 'a> {
             inner: &'a mut T,
-            error: Result<(), WriteAllError<T::Error>>,
+            error: Result<(), T::Error>,
         }
 
         impl<T: Write + ?Sized> fmt::Write for Adapter<'_, T> {
@@ -466,10 +429,7 @@ pub trait Write: ErrorType {
             Ok(()) => Ok(()),
             Err(..) => match output.error {
                 // check if the error came from the underlying `Write` or not
-                Err(e) => match e {
-                    WriteAllError::WriteZero => Err(WriteFmtError::WriteZero),
-                    WriteAllError::Other(e) => Err(WriteFmtError::Other(e)),
-                },
+                Err(e) => Err(WriteFmtError::Other(e)),
                 Ok(()) => Err(WriteFmtError::FmtError),
             },
         }