cleanup: Fix previously disabled clippy checks

There were not too many cases here. This fixes:

- comparison_chain
- wrong_self_convention
- upper_case_acronyms
- from_over_into
- let-and-return

The collapsible_if check is moved to the permanently
allowed checks. The cases we do have improve
readability or semantics.

BUG=chromium:908640
TEST=Kokoro

Change-Id: I6e905d08e2a87aa0862d4d1cf5ff57b60e95fa7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3278776
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
This commit is contained in:
Dennis Kempin 2021-11-12 14:42:28 -08:00 committed by Commit Bot
parent 18027ee80a
commit c3dedf3cc1
16 changed files with 123 additions and 112 deletions

View file

@ -8,13 +8,7 @@ rustflags = [
# "-Dwarnings",
# TODO(crbug/908640): To be resolved.
"-Aclippy::collapsible_if", # 4 errors
"-Aclippy::comparison_chain", # 1 errors
"-Aclippy::missing_safety_doc", # 26 errors
"-Aclippy::wrong_self_convention", # 8 errors
"-Aclippy::upper_case_acronyms", # 1 errors
"-Aclippy::from_over_into", # 1 errors
"-Aclippy::let-and-return", # 1 errors
# False positives affecting WlVfd @ `devices/src/virtio/wl.rs`.
# Bug: https://github.com/rust-lang/rust-clippy/issues/6312
@ -43,4 +37,5 @@ rustflags = [
"-Aclippy::useless_transmute",
"-Aclippy::new-ret-no-self",
"-Aclippy::result-unit-err",
"-Aclippy::collapsible_if",
]

View file

@ -547,7 +547,7 @@ impl PciCapability for MsixCap {
}
fn id(&self) -> PciCapabilityID {
PciCapabilityID::MSIX
PciCapabilityID::Msix
}
fn writable_bits(&self) -> Vec<u32> {

View file

@ -145,7 +145,7 @@ pub enum PciBridgeSubclass {
PcmciaBridge = 0x05,
NuBusBridge = 0x06,
CardBusBridge = 0x07,
RACEwayBridge = 0x08,
RaceWayBridge = 0x08,
PciToPciSemiTransparentBridge = 0x09,
InfiniBrandToPciHostBridge = 0x0a,
OtherBridgeDevice = 0x80,
@ -162,9 +162,9 @@ impl PciSubclass for PciBridgeSubclass {
#[derive(Copy, Clone)]
pub enum PciSerialBusSubClass {
Firewire = 0x00,
ACCESSbus = 0x01,
SSA = 0x02,
USB = 0x03,
AccessBus = 0x01,
Ssa = 0x02,
Usb = 0x03,
}
impl PciSubclass for PciSerialBusSubClass {
@ -190,21 +190,21 @@ pub enum PciCapabilityID {
VitalProductData = 0x03,
SlotIdentification = 0x04,
MessageSignalledInterrupts = 0x05,
CompactPCIHotSwap = 0x06,
PCIX = 0x07,
CompactPciHotSwap = 0x06,
Pcix = 0x07,
HyperTransport = 0x08,
VendorSpecific = 0x09,
Debugport = 0x0A,
CompactPCICentralResourceControl = 0x0B,
PCIStandardHotPlugController = 0x0C,
CompactPciCentralResourceControl = 0x0B,
PciStandardHotPlugController = 0x0C,
BridgeSubsystemVendorDeviceID = 0x0D,
AGPTargetPCIPCIbridge = 0x0E,
AgpTargetPciPciBridge = 0x0E,
SecureDevice = 0x0F,
PCIExpress = 0x10,
MSIX = 0x11,
SATADataIndexConf = 0x12,
PCIAdvancedFeatures = 0x13,
PCIEnhancedAllocation = 0x14,
PciExpress = 0x10,
Msix = 0x11,
SataDataIndexConf = 0x12,
PciAdvancedFeatures = 0x13,
PciEnhancedAllocation = 0x14,
}
/// A PCI capability list. Devices can optionally specify capabilities in their configuration space.
@ -230,7 +230,7 @@ pub struct PciConfiguration {
#[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)]
pub enum PciBarRegionType {
Memory32BitRegion = 0,
IORegion = 0x01,
IoRegion = 0x01,
Memory64BitRegion = 0x04,
}
@ -467,7 +467,7 @@ impl PciConfiguration {
let min_size = if config.is_expansion_rom() {
BAR_ROM_MIN_SIZE
} else if config.region_type == PciBarRegionType::IORegion {
} else if config.region_type == PciBarRegionType::IoRegion {
BAR_IO_MIN_SIZE
} else {
BAR_MEM_MIN_SIZE
@ -487,7 +487,7 @@ impl PciConfiguration {
.checked_add(config.size)
.ok_or(Error::BarAddressInvalid(config.addr, config.size))?;
match config.region_type {
PciBarRegionType::Memory32BitRegion | PciBarRegionType::IORegion => {
PciBarRegionType::Memory32BitRegion | PciBarRegionType::IoRegion => {
if end_addr > u64::from(u32::max_value()) {
return Err(Error::BarAddressInvalid(config.addr, config.size));
}
@ -520,7 +520,7 @@ impl PciConfiguration {
config.prefetchable as u32 | config.region_type as u32,
)
}
PciBarRegionType::IORegion => {
PciBarRegionType::IoRegion => {
self.registers[COMMAND_REG] |= COMMAND_REG_IO_SPACE_MASK;
(BAR_IO_ADDR_MASK, config.region_type as u32)
}
@ -585,7 +585,7 @@ impl PciConfiguration {
};
match bar_type {
PciBarRegionType::IORegion => u64::from(self.registers[bar_idx] & BAR_IO_ADDR_MASK),
PciBarRegionType::IoRegion => u64::from(self.registers[bar_idx] & BAR_IO_ADDR_MASK),
PciBarRegionType::Memory32BitRegion => {
u64::from(self.registers[bar_idx] & BAR_MEM_ADDR_MASK)
}
@ -701,7 +701,7 @@ impl PciBarConfiguration {
}
pub fn is_io(&self) -> bool {
self.region_type == PciBarRegionType::IORegion
self.region_type == PciBarRegionType::IoRegion
}
}
@ -1001,14 +1001,14 @@ mod tests {
PciBarConfiguration::new(
0,
0x4,
PciBarRegionType::IORegion,
PciBarRegionType::IoRegion,
PciBarPrefetchable::NotPrefetchable,
)
.set_address(0x1230),
)
.expect("add_pci_bar failed");
assert_eq!(cfg.get_bar_type(0), Some(PciBarRegionType::IORegion));
assert_eq!(cfg.get_bar_type(0), Some(PciBarRegionType::IoRegion));
assert_eq!(cfg.get_bar_addr(0), 0x1230);
assert_eq!(cfg.writable_bits[BAR0_REG], 0xFFFFFFFC);
@ -1019,7 +1019,7 @@ mod tests {
addr: 0x1230,
size: 0x4,
bar_idx: 0,
region_type: PciBarRegionType::IORegion,
region_type: PciBarRegionType::IoRegion,
prefetchable: PciBarPrefetchable::NotPrefetchable
})
);
@ -1070,7 +1070,7 @@ mod tests {
PciBarConfiguration::new(
3,
0x4,
PciBarRegionType::IORegion,
PciBarRegionType::IoRegion,
PciBarPrefetchable::NotPrefetchable,
)
.set_address(0x1230),
@ -1105,7 +1105,7 @@ mod tests {
addr: 0x1230,
size: 0x4,
bar_idx: 3,
region_type: PciBarRegionType::IORegion,
region_type: PciBarRegionType::IoRegion,
prefetchable: PciBarPrefetchable::NotPrefetchable
})
);
@ -1142,7 +1142,7 @@ mod tests {
addr: 0x1230,
size: 0x4,
bar_idx: 3,
region_type: PciBarRegionType::IORegion,
region_type: PciBarRegionType::IoRegion,
prefetchable: PciBarPrefetchable::NotPrefetchable
})
);
@ -1170,7 +1170,7 @@ mod tests {
PciBarConfiguration::new(
0,
0x2,
PciBarRegionType::IORegion,
PciBarRegionType::IoRegion,
PciBarPrefetchable::NotPrefetchable,
)
.set_address(0x1230),
@ -1184,7 +1184,7 @@ mod tests {
PciBarConfiguration::new(
0,
0x3,
PciBarRegionType::IORegion,
PciBarRegionType::IoRegion,
PciBarPrefetchable::NotPrefetchable,
)
.set_address(0x1230),
@ -1238,7 +1238,7 @@ mod tests {
cfg.add_pci_bar(PciBarConfiguration::new(
ROM_BAR_IDX,
0x1000,
PciBarRegionType::IORegion,
PciBarRegionType::IoRegion,
PciBarPrefetchable::NotPrefetchable,
),),
Err(Error::BarInvalidRomType)

View file

@ -414,7 +414,7 @@ mod tests {
PciBarConfiguration::new(
2,
BAR2_SIZE,
PciBarRegionType::IORegion,
PciBarRegionType::IoRegion,
PciBarPrefetchable::NotPrefetchable,
)
.set_address(BAR2_ADDR),

View file

@ -1,6 +1,7 @@
// Copyright 2021 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
use std::cmp::Ordering;
use std::sync::Arc;
use sync::Mutex;
@ -235,28 +236,36 @@ impl PciDevice for PciBridge {
// The driver is only allowed to do aligned, properly sized access.
let bar0 = self.config.get_bar_addr(self.setting_bar as usize);
let offset = addr - bar0;
if offset < BR_MSIX_PBA_OFFSET {
self.msix_config
.lock()
.read_msix_table(offset - BR_MSIX_TABLE_OFFSET, data);
} else if BR_MSIX_PBA_OFFSET == offset {
self.msix_config
.lock()
.read_pba_entries(offset - BR_MSIX_PBA_OFFSET, data);
match offset.cmp(&BR_MSIX_PBA_OFFSET) {
Ordering::Less => {
self.msix_config
.lock()
.read_msix_table(offset - BR_MSIX_TABLE_OFFSET, data);
}
Ordering::Equal => {
self.msix_config
.lock()
.read_pba_entries(offset - BR_MSIX_PBA_OFFSET, data);
}
Ordering::Greater => (),
}
}
fn write_bar(&mut self, addr: u64, data: &[u8]) {
let bar0 = self.config.get_bar_addr(self.setting_bar as usize);
let offset = addr - bar0;
if offset < BR_MSIX_PBA_OFFSET {
self.msix_config
.lock()
.write_msix_table(offset - BR_MSIX_TABLE_OFFSET, data);
} else if BR_MSIX_PBA_OFFSET == offset {
self.msix_config
.lock()
.write_pba_entries(offset - BR_MSIX_PBA_OFFSET, data);
match offset.cmp(&BR_MSIX_PBA_OFFSET) {
Ordering::Less => {
self.msix_config
.lock()
.write_msix_table(offset - BR_MSIX_TABLE_OFFSET, data);
}
Ordering::Equal => {
self.msix_config
.lock()
.write_pba_entries(offset - BR_MSIX_PBA_OFFSET, data);
}
Ordering::Greater => (),
}
}
}
@ -345,7 +354,7 @@ impl PciCapability for PcieCap {
}
fn id(&self) -> PciCapabilityID {
PciCapabilityID::PCIExpress
PciCapabilityID::PciExpress
}
fn writable_bits(&self) -> Vec<u32> {
@ -545,7 +554,7 @@ impl PcieDevice for PcieRootPort {
}
fn set_capability_reg_idx(&mut self, id: PciCapabilityID, reg_idx: usize) {
if let PciCapabilityID::PCIExpress = id {
if let PciCapabilityID::PciExpress = id {
self.pcie_cap_reg_idx = Some(reg_idx)
}
}

View file

@ -1000,7 +1000,7 @@ impl PciDevice for VfioPciDevice {
self.io_regions.push(PciBarConfiguration::new(
i as usize,
u64::from(size),
PciBarRegionType::IORegion,
PciBarRegionType::IoRegion,
PciBarPrefetchable::NotPrefetchable,
));
}

View file

@ -107,7 +107,7 @@ impl XhciController {
0x01b73, // fresco logic, (google = 0x1ae0)
0x1000, // fresco logic pdk. This chip has broken msi. See kernel xhci-pci.c
PciClassCode::SerialBusController,
&PciSerialBusSubClass::USB,
&PciSerialBusSubClass::Usb,
Some(&UsbControllerProgrammingInterface::Usb3HostController),
PciHeaderType::Device,
false,

View file

@ -762,21 +762,21 @@ pub struct VioSStreamParams {
pub rate: u8,
}
impl Into<virtio_snd_pcm_set_params> for (u32, VioSStreamParams) {
fn into(self) -> virtio_snd_pcm_set_params {
impl From<(u32, VioSStreamParams)> for virtio_snd_pcm_set_params {
fn from(val: (u32, VioSStreamParams)) -> Self {
virtio_snd_pcm_set_params {
hdr: virtio_snd_pcm_hdr {
hdr: virtio_snd_hdr {
code: VIRTIO_SND_R_PCM_SET_PARAMS.into(),
},
stream_id: self.0.into(),
stream_id: val.0.into(),
},
buffer_bytes: self.1.buffer_bytes.into(),
period_bytes: self.1.period_bytes.into(),
features: self.1.features.into(),
channels: self.1.channels,
format: self.1.format,
rate: self.1.rate,
buffer_bytes: val.1.buffer_bytes.into(),
period_bytes: val.1.period_bytes.into(),
features: val.1.features.into(),
channels: val.1.channels,
format: val.1.format,
rate: val.1.rate,
padding: 0u8,
}
}

View file

@ -743,20 +743,20 @@ pub struct SetattrIn {
}
unsafe impl DataInit for SetattrIn {}
impl Into<libc::stat64> for SetattrIn {
fn into(self) -> libc::stat64 {
impl From<SetattrIn> for libc::stat64 {
fn from(s: SetattrIn) -> libc::stat64 {
// Safe because we are zero-initializing a struct with only POD fields.
let mut out: libc::stat64 = unsafe { mem::zeroed() };
out.st_mode = self.mode;
out.st_uid = self.uid;
out.st_gid = self.gid;
out.st_size = self.size as i64;
out.st_atime = self.atime as libc::time_t;
out.st_mtime = self.mtime as libc::time_t;
out.st_ctime = self.ctime as libc::time_t;
out.st_atime_nsec = self.atimensec as libc::c_long;
out.st_mtime_nsec = self.mtimensec as libc::c_long;
out.st_ctime_nsec = self.ctimensec as libc::c_long;
out.st_mode = s.mode;
out.st_uid = s.uid;
out.st_gid = s.gid;
out.st_size = s.size as i64;
out.st_atime = s.atime as libc::time_t;
out.st_mtime = s.mtime as libc::time_t;
out.st_ctime = s.ctime as libc::time_t;
out.st_atime_nsec = s.atimensec as libc::c_long;
out.st_mtime_nsec = s.mtimensec as libc::c_long;
out.st_ctime_nsec = s.ctimensec as libc::c_long;
out
}

View file

@ -4,6 +4,8 @@
//! Generated using ./xlib_generator.sh
#![allow(clippy::upper_case_acronyms)]
#[link(name = "X11")]
extern "C" {}

View file

@ -12,6 +12,8 @@ cat >xlib.rs <<EOF
//! Generated using ./xlib_generator.sh
#![allow(clippy::upper_case_acronyms)]
#[link(name = "X11")]
extern "C" {}

View file

@ -4,7 +4,7 @@
//! rutabaga_2d: Handles 2D virtio-gpu hypercalls.
use std::cmp::{max, min};
use std::cmp::{max, min, Ordering};
use data_model::*;
@ -84,15 +84,19 @@ pub fn transfer_2d<'a, S: Iterator<Item = VolatileSlice<'a>>>(
let offset_within_src = src_copyable_start_offset.saturating_sub(src_start_offset);
if src_line_end_offset > src_end_offset {
next_src = true;
next_line = false;
} else if src_line_end_offset == src_end_offset {
next_src = true;
next_line = true;
} else {
next_src = false;
next_line = true;
match src_line_end_offset.cmp(&src_end_offset) {
Ordering::Greater => {
next_src = true;
next_line = false;
}
Ordering::Equal => {
next_src = true;
next_line = true;
}
Ordering::Less => {
next_src = false;
next_line = true;
}
}
let src_subslice = src.get_slice(offset_within_src as usize, copyable_size as usize)?;

View file

@ -275,6 +275,7 @@ impl RutabagaGralloc {
// towards the Vulkan api. This function allows for a variety of quirks, but for now just
// choose the most shiny backend that the user has built. The rationale is "why would you
// build it if you don't want to use it".
#[allow(clippy::let_and_return)]
let mut _backend = GrallocBackend::System;
#[cfg(feature = "minigbm")]

View file

@ -2089,12 +2089,10 @@ fn validate_arguments(cfg: &mut Config) -> std::result::Result<(), argument::Err
}
}
#[cfg(all(target_arch = "x86_64", feature = "gdb"))]
if cfg.gdb.is_some() {
if cfg.vcpu_count.unwrap_or(1) != 1 {
return Err(argument::Error::ExpectedArgument(
"`gdb` requires the number of vCPU to be 1".to_owned(),
));
}
if cfg.gdb.is_some() && cfg.vcpu_count.unwrap_or(1) != 1 {
return Err(argument::Error::ExpectedArgument(
"`gdb` requires the number of vCPU to be 1".to_owned(),
));
}
if cfg.host_cpu_topology {
// Safe because we pass a flag for this call and the host supports this system call

View file

@ -11,7 +11,7 @@ use base::error;
use data_model::DataInit;
use vm_memory::{GuestAddress, GuestMemory};
pub struct ACPIDevResource {
pub struct AcpiDevResource {
pub amls: Vec<u8>,
pub pm_iobase: u64,
/// Additional system descriptor tables.
@ -20,7 +20,7 @@ pub struct ACPIDevResource {
#[repr(C)]
#[derive(Clone, Copy, Default)]
struct LocalAPIC {
struct LocalApic {
_type: u8,
_length: u8,
_processor_id: u8,
@ -29,11 +29,11 @@ struct LocalAPIC {
}
// Safe as LocalAPIC structure only contains raw data
unsafe impl DataInit for LocalAPIC {}
unsafe impl DataInit for LocalApic {}
#[repr(C)]
#[derive(Clone, Copy, Default)]
struct IOAPIC {
struct Ioapic {
_type: u8,
_length: u8,
_ioapic_id: u8,
@ -43,11 +43,11 @@ struct IOAPIC {
}
// Safe as IOAPIC structure only contains raw data
unsafe impl DataInit for IOAPIC {}
unsafe impl DataInit for Ioapic {}
#[repr(C)]
#[derive(Clone, Copy, Default)]
struct Localx2APIC {
struct Localx2Apic {
_type: u8,
_length: u8,
_reserved: u16,
@ -57,7 +57,7 @@ struct Localx2APIC {
}
// Safe as LocalAPIC structure only contains raw data
unsafe impl DataInit for Localx2APIC {}
unsafe impl DataInit for Localx2Apic {}
const OEM_REVISION: u32 = 1;
//DSDT
@ -222,9 +222,9 @@ fn sync_acpi_id_from_cpuid(
// the host.
has_leafb = true;
let x2apic = Localx2APIC {
let x2apic = Localx2Apic {
_type: MADT_TYPE_LOCAL_X2APIC,
_length: std::mem::size_of::<Localx2APIC>() as u8,
_length: std::mem::size_of::<Localx2Apic>() as u8,
_x2apic_id: cpuid_entry.edx,
_flags: MADT_ENABLED,
_processor_id: (vcpu + 1) as u32,
@ -244,9 +244,9 @@ fn sync_acpi_id_from_cpuid(
apic_id = (cpuid_entry.ebx >> CPUID_LEAF0_EBX_CPUID_SHIFT & 0xff) as u8;
}
let apic = LocalAPIC {
let apic = LocalApic {
_type: MADT_TYPE_LOCAL_APIC,
_length: std::mem::size_of::<LocalAPIC>() as u8,
_length: std::mem::size_of::<LocalApic>() as u8,
_processor_id: vcpu as u8,
_apic_id: apic_id,
_flags: MADT_ENABLED,
@ -282,7 +282,7 @@ pub fn create_acpi_tables(
guest_mem: &GuestMemory,
num_cpus: u8,
sci_irq: u32,
acpi_dev_resource: ACPIDevResource,
acpi_dev_resource: AcpiDevResource,
host_cpus: Option<VcpuAffinity>,
apic_ids: &mut Vec<usize>,
) -> Option<GuestAddress> {
@ -365,9 +365,9 @@ pub fn create_acpi_tables(
}
_ => {
for cpu in 0..num_cpus {
let apic = LocalAPIC {
let apic = LocalApic {
_type: MADT_TYPE_LOCAL_APIC,
_length: std::mem::size_of::<LocalAPIC>() as u8,
_length: std::mem::size_of::<LocalApic>() as u8,
_processor_id: cpu,
_apic_id: cpu,
_flags: MADT_ENABLED,
@ -378,9 +378,9 @@ pub fn create_acpi_tables(
}
}
madt.append(IOAPIC {
madt.append(Ioapic {
_type: MADT_TYPE_IO_APIC,
_length: std::mem::size_of::<IOAPIC>() as u8,
_length: std::mem::size_of::<Ioapic>() as u8,
_apic_address: super::mptable::IO_APIC_DEFAULT_PHYS_BASE,
..Default::default()
});

View file

@ -1096,7 +1096,7 @@ impl X8664arch {
battery: (&Option<BatteryType>, Option<Minijail>),
mmio_bus: &devices::Bus,
resume_notify_devices: &mut Vec<Arc<Mutex<dyn BusResumeDevice>>>,
) -> Result<(acpi::ACPIDevResource, Option<BatControl>)> {
) -> Result<(acpi::AcpiDevResource, Option<BatControl>)> {
// The AML data for the acpi devices
let mut amls = Vec::new();
@ -1185,7 +1185,7 @@ impl X8664arch {
};
Ok((
acpi::ACPIDevResource {
acpi::AcpiDevResource {
amls,
pm_iobase,
sdts,