Skip to content

Commit b6d30ff

Browse files
committed
uefi: serial: improve documentation and correctness of read() and write()
Next to documentation improvements, I replaced the debug assertions with runtime assertions. If the underlying protocol implementation doesn't behave as expected, we should at least fail hard. I tested this with OVMF in QEMU 10.x and also on real hardware.
1 parent e35404f commit b6d30ff

File tree

1 file changed

+55
-16
lines changed

1 file changed

+55
-16
lines changed

uefi/src/proto/console/serial.rs

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
55
use crate::proto::unsafe_protocol;
66
use crate::{Error, Result, Status, StatusExt};
7+
use core::fmt;
78
use core::fmt::Write;
89
use uefi_raw::protocol::console::serial::{
910
SerialIoProtocol, SerialIoProtocol_1_1, SerialIoProtocolRevision,
@@ -131,28 +132,66 @@ impl Serial {
131132
unsafe { (self.0.set_control_bits)(&mut self.0, bits) }.to_result()
132133
}
133134

134-
/// Reads data from this device.
135+
/// Reads data from the device. This function has the raw semantics of the
136+
/// underlying UEFI protocol.
135137
///
136-
/// This operation will block until the buffer has been filled with data or
137-
/// an error occurs. In the latter case, the error will indicate how many
138-
/// bytes were actually read from the device.
139-
pub fn read(&mut self, data: &mut [u8]) -> Result<(), usize> {
140-
let mut buffer_size = data.len();
141-
unsafe { (self.0.read)(&mut self.0, &mut buffer_size, data.as_mut_ptr()) }.to_result_with(
142-
|| debug_assert_eq!(buffer_size, data.len()),
138+
/// The function will read bytes until either the buffer is full or a
139+
/// timeout or overrun error occurs.
140+
///
141+
/// # Arguments
142+
///
143+
/// - `buffer`: buffer to fill
144+
///
145+
/// # Tips
146+
///
147+
/// Consider setting non-default properties via [`Self::set_attributes`]
148+
/// and [`Self::set_control_bits`] matching your use-case. For more info,
149+
/// please read the general [documentation](Self) of the protocol.
150+
///
151+
/// # Errors
152+
///
153+
/// - [`Status::DEVICE_ERROR`]: serial device reported an error
154+
/// - [`Status::TIMEOUT`]: operation was stopped due to a timeout or overrun
155+
pub fn read(&mut self, buffer: &mut [u8]) -> Result<(), usize /* read bytes on timeout*/> {
156+
let mut buffer_size = buffer.len();
157+
unsafe { (self.0.read)(&mut self.0, &mut buffer_size, buffer.as_mut_ptr()) }.to_result_with(
158+
|| {
159+
// By spec: Either reads all requested bytes (and blocks) or
160+
// returns early with an error.
161+
assert_eq!(buffer_size, buffer.len())
162+
},
143163
|_| buffer_size,
144164
)
145165
}
146166

147-
/// Writes data to this device.
167+
/// Writes data to this device. This function has the raw semantics of the
168+
/// underlying UEFI protocol.
169+
///
170+
/// The function will try to write all provided bytes in the configured
171+
/// timeout.
172+
///
173+
/// # Arguments
174+
///
175+
/// - `data`: bytes to write
176+
///
177+
/// # Tips
178+
///
179+
/// Consider setting non-default properties via [`Self::set_attributes`]
180+
/// and [`Self::set_control_bits`] matching your use-case. For more info,
181+
/// please read the general [documentation](Self) of the protocol.
182+
///
183+
/// # Errors
148184
///
149-
/// This operation will block until the data has been fully written or an
150-
/// error occurs. In the latter case, the error will indicate how many bytes
151-
/// were actually written to the device.
152-
pub fn write(&mut self, data: &[u8]) -> Result<(), usize> {
185+
/// - [`Status::DEVICE_ERROR`]: serial device reported an error
186+
/// - [`Status::TIMEOUT`]: data write was stopped due to a timeout
187+
pub fn write(&mut self, data: &[u8]) -> Result<(), usize /* bytes written on timeout */> {
153188
let mut buffer_size = data.len();
154189
unsafe { (self.0.write)(&mut self.0, &mut buffer_size, data.as_ptr()) }.to_result_with(
155-
|| debug_assert_eq!(buffer_size, data.len()),
190+
|| {
191+
// By spec: Either reads all requested bytes (and blocks) or
192+
// returns early with an error.
193+
assert_eq!(buffer_size, data.len())
194+
},
156195
|_| buffer_size,
157196
)
158197
}
@@ -199,7 +238,7 @@ impl Serial {
199238
}
200239

201240
impl Write for Serial {
202-
fn write_str(&mut self, s: &str) -> core::fmt::Result {
203-
self.write(s.as_bytes()).map_err(|_| core::fmt::Error)
241+
fn write_str(&mut self, s: &str) -> fmt::Result {
242+
self.write(s.as_bytes()).map(|_| ()).map_err(|_| fmt::Error)
204243
}
205244
}

0 commit comments

Comments
 (0)