Skip to content

Commit

Permalink
Clarify write() may only return Ok(0) if data is empty.
Browse files Browse the repository at this point in the history
In other situations an error should be returned instead;
update ErrorKind to add this and update out slice_mut implementation
to use it.
  • Loading branch information
adamgreig committed Sep 13, 2023
1 parent 030a232 commit 72f6e78
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 59 deletions.
12 changes: 11 additions & 1 deletion embedded-io/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
37 changes: 33 additions & 4 deletions embedded-io/src/impls/slice_mut.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,52 @@
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.
///
/// 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;
Expand Down
68 changes: 14 additions & 54 deletions embedded-io/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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:?}")
Expand All @@ -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.
Expand All @@ -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`].
Expand Down Expand Up @@ -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<usize, Self::Error>;

/// Flush this output stream, blocking until all intermediately buffered contents reach their destination.
Expand All @@ -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(())
Expand All @@ -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> {
Expand All @@ -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),
},
}
Expand Down

0 comments on commit 72f6e78

Please sign in to comment.