Skip to content

Commit 2631911

Browse files
orthrosCQ Bot
authored andcommitted
[fastboot]: fix clippy errors
Return errors on short writes. Fun fact: we cannot use `write_all` for this as in TCP Fastboot mode, we write _more_ than the passed buffer (the first 8 bytes we write is the length of the actual message we are sending), and `write_all` will panic as it thinkss we are trying a buffer overflow as we report back that we wrote more than we sent. See: https://github.com/rust-lang/futures-rs/blob/master/futures-util/src/io/write_all.rs#L33 And https://doc.rust-lang.org/src/core/slice/mod.rs.html#1886 Instead report back if there are short writes and error Fixed: b/42177040 Bug: b/42177040 Change-Id: I048cd4d9087168263aec5a4f7576e44341ca67bd Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1036216 Reviewed-by: Andrew Davies <awdavies@google.com> Fuchsia-Auto-Submit: Colin Nelson <colnnelson@google.com> Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com> Reviewed-by: Casey Dahlin <sadmac@google.com>
1 parent 8d8047c commit 2631911

File tree

4 files changed

+31
-7
lines changed

4 files changed

+31
-7
lines changed

src/developer/fastboot/src/lib.rs

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ lazy_static! {
3535
pub enum SendError {
3636
#[error("timed out reading a reply from device")]
3737
Timeout,
38+
#[error("Did not write correct number of bytes. Got {:?}. Want {}", written, expected)]
39+
ShortWrite { written: usize, expected: usize },
3840
}
3941

4042
#[derive(Debug, Error)]
@@ -171,7 +173,6 @@ async fn read_with_timeout<T: AsyncRead + Unpin>(
171173
}
172174
}
173175

174-
#[allow(clippy::unused_io_amount)] // TODO(https://fxbug.dev/42177040)
175176
pub async fn send_with_listener<T: AsyncRead + AsyncWrite + Unpin>(
176177
cmd: Command,
177178
interface: &mut T,
@@ -180,24 +181,28 @@ pub async fn send_with_listener<T: AsyncRead + AsyncWrite + Unpin>(
180181
let _lock = SEND_LOCK.lock().await;
181182
let bytes = Vec::<u8>::try_from(&cmd)?;
182183
tracing::debug!("Fastboot: writing command {cmd:?}: {}", String::from_utf8_lossy(&bytes));
183-
interface.write(&bytes).await?;
184+
let written = interface.write(&bytes).await?;
185+
if written < bytes.len() {
186+
return Err(anyhow!(SendError::ShortWrite { written, expected: bytes.len() }));
187+
}
184188
read(interface, listener).await
185189
}
186190

187191
#[tracing::instrument(skip(interface))]
188-
#[allow(clippy::unused_io_amount)] // TODO(https://fxbug.dev/42177040)
189192
pub async fn send<T: AsyncRead + AsyncWrite + Unpin>(
190193
cmd: Command,
191194
interface: &mut T,
192195
) -> Result<Reply> {
193196
let _lock = SEND_LOCK.lock().await;
194197
let bytes = Vec::<u8>::try_from(&cmd)?;
195198
tracing::debug!("Fastboot: writing command {cmd:?}: {}", String::from_utf8_lossy(&bytes));
196-
interface.write(&bytes).await?;
199+
let written = interface.write(&bytes).await?;
200+
if written < bytes.len() {
201+
return Err(anyhow!(SendError::ShortWrite { written, expected: bytes.len() }));
202+
}
197203
read_and_log_info(interface).await
198204
}
199205

200-
#[allow(clippy::unused_io_amount)] // TODO(https://fxbug.dev/42177040)
201206
pub async fn send_with_timeout<T: AsyncRead + AsyncWrite + Unpin>(
202207
cmd: Command,
203208
interface: &mut T,
@@ -206,7 +211,10 @@ pub async fn send_with_timeout<T: AsyncRead + AsyncWrite + Unpin>(
206211
let _lock = SEND_LOCK.lock().await;
207212
let bytes = Vec::<u8>::try_from(&cmd)?;
208213
tracing::debug!("Fastboot: writing command {cmd:?}: {}", String::from_utf8_lossy(&bytes));
209-
interface.write(&bytes).await?;
214+
let written = interface.write(&bytes).await?;
215+
if written < bytes.len() {
216+
return Err(anyhow!(SendError::ShortWrite { written, expected: bytes.len() }));
217+
}
210218
read_with_timeout(interface, &LogInfoListener {}, timeout).await
211219
}
212220

@@ -263,7 +271,13 @@ pub async fn upload_with_read_timeout<T: AsyncRead + AsyncWrite + Unpin, R: Read
263271
listener.on_error(&err).await?;
264272
bail!(err);
265273
}
266-
_ => {
274+
Ok(written) => {
275+
if written < n {
276+
return Err(anyhow!(SendError::ShortWrite {
277+
written,
278+
expected: n
279+
}));
280+
}
267281
listener.on_progress(n.try_into().unwrap()).await?;
268282
tracing::trace!("fastboot: wrote {} bytes", n);
269283
}

src/developer/ffx/lib/fastboot/interface/src/fastboot_interface.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ pub enum FastbootError {
111111

112112
#[error("Failed to get all variables: {}", .0 )]
113113
GetAllVarsFailed(String),
114+
115+
#[error("Did not write correct number of bytes. Got {:?}. Want {}", written, expected)]
116+
ShortWrite { written: usize, expected: usize },
114117
}
115118

116119
#[derive(thiserror::Error, Debug)]

src/developer/ffx/lib/fastboot/interface/src/fastboot_proxy.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,12 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Debug> Fastboot for FastbootProxy<T> {
278278
};
279279
Err(FastbootError::FlashError(FlashError::TimeoutError(message)))
280280
}
281+
SendError::ShortWrite { written, expected } => {
282+
Err(FastbootError::ShortWrite {
283+
written: *written,
284+
expected: *expected,
285+
})
286+
}
281287
}
282288
} else {
283289
Err(FastbootError::FlashError(

src/developer/ffx/tests/target/src/tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ async fn enter_fastboot(
279279
let line = res.unwrap();
280280
eprintln!("{}", line);
281281
if line.contains("Press f to enter fastboot.") {
282+
eprintln!("TEST: About to press f");
282283
writer.write_all(b"f").await.expect("failed to press f");
283284
break;
284285
}

0 commit comments

Comments
 (0)