Skip to content

Commit 4a11487

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 4a11487

1 file changed

Lines changed: 55 additions & 15 deletions

File tree

uefi/src/proto/console/serial.rs

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -131,28 +131,66 @@ impl Serial {
131131
unsafe { (self.0.set_control_bits)(&mut self.0, bits) }.to_result()
132132
}
133133

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

147-
/// Writes data to this device.
166+
/// Writes data to this device. This function has the raw semantics of the
167+
/// underlying UEFI protocol.
168+
///
169+
/// The function will try to write all provided bytes in the configured
170+
/// timeout.
171+
///
172+
/// # Arguments
173+
///
174+
/// - `data`: bytes to write
175+
///
176+
/// # Tips
177+
///
178+
/// Consider setting non-default properties via [`Self::set_attributes`]
179+
/// and [`Self::set_control_bits`] matching your use-case. For more info,
180+
/// please read the general [documentation](Self) of the protocol.
181+
///
182+
/// # Errors
148183
///
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> {
184+
/// - [`Status::DEVICE_ERROR`]: serial device reported an error
185+
/// - [`Status::TIMEOUT`]: data write was stopped due to a timeout
186+
pub fn write(&mut self, data: &[u8]) -> Result<(), usize /* bytes written on timeout */> {
153187
let mut buffer_size = data.len();
154188
unsafe { (self.0.write)(&mut self.0, &mut buffer_size, data.as_ptr()) }.to_result_with(
155-
|| debug_assert_eq!(buffer_size, data.len()),
189+
|| {
190+
// By spec: Either reads all requested bytes (and blocks) or
191+
// returns early with an error.
192+
assert_eq!(buffer_size, data.len())
193+
},
156194
|_| buffer_size,
157195
)
158196
}
@@ -200,6 +238,8 @@ impl Serial {
200238

201239
impl Write for Serial {
202240
fn write_str(&mut self, s: &str) -> core::fmt::Result {
203-
self.write(s.as_bytes()).map_err(|_| core::fmt::Error)
241+
self.write(s.as_bytes())
242+
.map(|_| ())
243+
.map_err(|_| core::fmt::Error)
204244
}
205245
}

0 commit comments

Comments
 (0)