From 8f3a23216016cd14c0d91676cb1f4611e0945f17 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 30 Nov 2018 17:11:35 -0800 Subject: [PATCH] linux: Clean up a misleading loop The `while sig_ok` in the original code suggests that `sig_ok` would be mutated by the loop body, but it was not. Really `while sig_ok` was being used to mean `if sig_ok { loop { ... } }`, with breaks to exit the loop body. I replaced `while sig_ok` with `if sig_ok` containing `loop`. Since this is an extra layer of indentation, I removed two layers of indentation by flattening a a nested match so the new code is overall less indented than before. Clippy flags such loops in which the loop condition never changes as high confidence of being a bug or at least misleading: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition TEST=run linux Change-Id: Ib925bbedbdda11bb50e47f8dd55c2f5af7c53698 Reviewed-on: https://chromium-review.googlesource.com/1357699 Commit-Ready: David Tolnay Tested-by: David Tolnay Reviewed-by: Zach Reizner --- src/linux.rs | 117 +++++++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 60 deletions(-) diff --git a/src/linux.rs b/src/linux.rs index c208e3b76b..f50cda5efa 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -659,71 +659,68 @@ fn run_vcpu( start_barrier.wait(); - while sig_ok { - let run_res = vcpu.run(); - match run_res { - Ok(run) => { - match run { - VcpuExit::IoIn { port, mut size } => { - let mut data = [0; 8]; - if size > data.len() { - error!("unsupported IoIn size of {} bytes", size); - size = data.len(); - } - io_bus.read(port as u64, &mut data[..size]); - if let Err(e) = vcpu.set_data(&data[..size]) { - error!("failed to set return data for IoIn: {:?}", e); - } + if sig_ok { + loop { + match vcpu.run() { + Ok(VcpuExit::IoIn { port, mut size }) => { + let mut data = [0; 8]; + if size > data.len() { + error!("unsupported IoIn size of {} bytes", size); + size = data.len(); } - VcpuExit::IoOut { - port, - mut size, - data, - } => { - if size > data.len() { - error!("unsupported IoOut size of {} bytes", size); - size = data.len(); - } - io_bus.write(port as u64, &data[..size]); + io_bus.read(port as u64, &mut data[..size]); + if let Err(e) = vcpu.set_data(&data[..size]) { + error!("failed to set return data for IoIn: {:?}", e); } - VcpuExit::MmioRead { address, mut size } => { - let mut data = [0; 8]; - mmio_bus.read(address, &mut data[..size]); - // Setting data for mmio can not fail. - let _ = vcpu.set_data(&data[..size]); - } - VcpuExit::MmioWrite { - address, - size, - data, - } => { - mmio_bus.write(address, &data[..size]); - } - VcpuExit::Hlt => break, - VcpuExit::Shutdown => break, - VcpuExit::SystemEvent(_, _) => - //TODO handle reboot and crash events - { - kill_signaled.store(true, Ordering::SeqCst) - } - r => warn!("unexpected vcpu exit: {:?}", r), } + Ok(VcpuExit::IoOut { + port, + mut size, + data, + }) => { + if size > data.len() { + error!("unsupported IoOut size of {} bytes", size); + size = data.len(); + } + io_bus.write(port as u64, &data[..size]); + } + Ok(VcpuExit::MmioRead { address, size }) => { + let mut data = [0; 8]; + mmio_bus.read(address, &mut data[..size]); + // Setting data for mmio can not fail. + let _ = vcpu.set_data(&data[..size]); + } + Ok(VcpuExit::MmioWrite { + address, + size, + data, + }) => { + mmio_bus.write(address, &data[..size]); + } + Ok(VcpuExit::Hlt) => break, + Ok(VcpuExit::Shutdown) => break, + Ok(VcpuExit::SystemEvent(_, _)) => + //TODO handle reboot and crash events + { + kill_signaled.store(true, Ordering::SeqCst) + } + Ok(r) => warn!("unexpected vcpu exit: {:?}", r), + Err(e) => match e.errno() { + libc::EAGAIN | libc::EINTR => {} + _ => { + error!("vcpu hit unknown error: {:?}", e); + break; + } + }, + } + if kill_signaled.load(Ordering::SeqCst) { + break; } - Err(e) => match e.errno() { - libc::EAGAIN | libc::EINTR => {} - _ => { - error!("vcpu hit unknown error: {:?}", e); - break; - } - }, - } - if kill_signaled.load(Ordering::SeqCst) { - break; - } - // Try to clear the signal that we use to kick VCPU if it is - // pending before attempting to handle pause requests. - clear_signal(SIGRTMIN() + 0).expect("failed to clear pending signal"); + // Try to clear the signal that we use to kick VCPU if it is + // pending before attempting to handle pause requests. + clear_signal(SIGRTMIN() + 0).expect("failed to clear pending signal"); + } } exit_evt .write(1)