From 72f6e782417109e3d54ea0eac14264a9ef3a9fc5 Mon Sep 17 00:00:00 2001 From: Adam Greig Date: Wed, 13 Sep 2023 02:08:26 +0100 Subject: [PATCH] Clarify write() may only return Ok(0) if data is empty. In other situations an error should be returned instead; update ErrorKind to add this and update out slice_mut implementation to use it. --- embedded-io/CHANGELOG.md | 12 +++++- embedded-io/src/impls/slice_mut.rs | 37 ++++++++++++++-- embedded-io/src/lib.rs | 68 ++++++------------------------ 3 files changed, 58 insertions(+), 59 deletions(-) diff --git a/embedded-io/CHANGELOG.md b/embedded-io/CHANGELOG.md index 937a43140..f5a60a3df 100644 --- a/embedded-io/CHANGELOG.md +++ b/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 \ No newline at end of file +- First release diff --git a/embedded-io/src/impls/slice_mut.rs b/embedded-io/src/impls/slice_mut.rs index b89b72182..d467613d8 100644 --- a/embedded-io/src/impls/slice_mut.rs +++ b/embedded-io/src/impls/slice_mut.rs @@ -1,10 +1,37 @@ -use crate::{ErrorType, Write}; +use crate::{Error, ErrorKind, ErrorType, Write}; use core::mem; +/// 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 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. /// @@ -12,12 +39,14 @@ impl ErrorType for &mut [u8] { /// 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 { 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; diff --git a/embedded-io/src/lib.rs b/embedded-io/src/lib.rs index 1422a7b4b..ba84628e4 100644 --- a/embedded-io/src/lib.rs +++ b/embedded-io/src/lib.rs @@ -61,7 +61,6 @@ impl From 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> for std::io::Error { } } -#[cfg(feature = "std")] -#[cfg_attr(docsrs, doc(cfg(feature = "std")))] -impl From> for std::io::Error { - fn from(err: WriteAllError) -> 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 fmt::Display for ReadExactError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{self:?}") @@ -275,8 +263,6 @@ impl std::error::Error for ReadExactError {} #[derive(Debug, Copy, Clone, Eq, PartialEq)] #[cfg_attr(feature = "defmt-03", derive(defmt::Format))] pub enum WriteFmtError { - /// [`Write::write`] wrote zero bytes - WriteZero, /// An error was encountered while formatting. FmtError, /// Error returned by the inner Write. @@ -299,32 +285,6 @@ impl fmt::Display for WriteFmtError { #[cfg_attr(docsrs, doc(cfg(feature = "std")))] impl std::error::Error for WriteFmtError {} -/// Error returned by [`Write::write_all`] -#[derive(Debug, Copy, Clone, Eq, PartialEq)] -#[cfg_attr(feature = "defmt-03", derive(defmt::Format))] -pub enum WriteAllError { - /// [`Write::write`] wrote zero bytes - WriteZero, - /// Error returned by the inner Write. - Other(E), -} - -impl From for WriteAllError { - fn from(err: E) -> Self { - Self::Other(err) - } -} - -impl fmt::Display for WriteAllError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{self:?}") - } -} - -#[cfg(feature = "std")] -#[cfg_attr(docsrs, doc(cfg(feature = "std")))] -impl std::error::Error for WriteAllError {} - /// 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 never return `Ok(0)` when `buf.is_empty()`. Situations where the writer is not + /// able to accept more bytes and likely never will must be indicated with errors, where the + /// `ErrorKind` is typically `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; /// 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> { + /// + /// 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>, + error: Result<(), T::Error>, } impl 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), }, }