Skip to content

Commit

Permalink
chore(clippy): update code with new lints
Browse files Browse the repository at this point in the history
With a new Rust version, new Clippy entered our repository.
This commit aligns our codebase with guidance provided by
new lints.

Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
  • Loading branch information
ShadowCurse committed Nov 28, 2024
1 parent 41bda16 commit bbadfbf
Show file tree
Hide file tree
Showing 36 changed files with 103 additions and 125 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ resolver = "2"

[workspace.lints.rust]
missing_debug_implementations = "warn"
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(kani)'] }

[workspace.lints.clippy]
ptr_as_ptr = "warn"
Expand Down
3 changes: 1 addition & 2 deletions src/firecracker/src/api_server/request/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ struct ActionBody {

pub(crate) fn parse_put_actions(body: &Body) -> Result<ParsedRequest, RequestError> {
METRICS.put_api_requests.actions_count.inc();
let action_body = serde_json::from_slice::<ActionBody>(body.raw()).map_err(|err| {
let action_body = serde_json::from_slice::<ActionBody>(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.actions_fails.inc();
err
})?;

match action_body.action_type {
Expand Down
3 changes: 1 addition & 2 deletions src/firecracker/src/api_server/request/boot_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ use super::Body;
pub(crate) fn parse_put_boot_source(body: &Body) -> Result<ParsedRequest, RequestError> {
METRICS.put_api_requests.boot_source_count.inc();
Ok(ParsedRequest::new_sync(VmmAction::ConfigureBootSource(
serde_json::from_slice::<BootSourceConfig>(body.raw()).map_err(|err| {
serde_json::from_slice::<BootSourceConfig>(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.boot_source_fails.inc();
err
})?,
)))
}
Expand Down
6 changes: 2 additions & 4 deletions src/firecracker/src/api_server/request/drive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ pub(crate) fn parse_put_drive(
return Err(RequestError::EmptyID);
};

let device_cfg = serde_json::from_slice::<BlockDeviceConfig>(body.raw()).map_err(|err| {
let device_cfg = serde_json::from_slice::<BlockDeviceConfig>(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.drive_fails.inc();
err
})?;

if id != device_cfg.drive_id {
Expand Down Expand Up @@ -51,9 +50,8 @@ pub(crate) fn parse_patch_drive(
};

let block_device_update_cfg: BlockDeviceUpdateConfig =
serde_json::from_slice::<BlockDeviceUpdateConfig>(body.raw()).map_err(|err| {
serde_json::from_slice::<BlockDeviceUpdateConfig>(body.raw()).inspect_err(|_| {
METRICS.patch_api_requests.drive_fails.inc();
err
})?;

if id != block_device_update_cfg.drive_id {
Expand Down
3 changes: 1 addition & 2 deletions src/firecracker/src/api_server/request/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ use super::Body;
pub(crate) fn parse_put_logger(body: &Body) -> Result<ParsedRequest, RequestError> {
METRICS.put_api_requests.logger_count.inc();
let res = serde_json::from_slice::<vmm::logger::LoggerConfig>(body.raw());
let config = res.map_err(|err| {
let config = res.inspect_err(|_| {
METRICS.put_api_requests.logger_fails.inc();
err
})?;
Ok(ParsedRequest::new_sync(VmmAction::ConfigureLogger(config)))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ pub(crate) fn parse_get_machine_config() -> Result<ParsedRequest, RequestError>

pub(crate) fn parse_put_machine_config(body: &Body) -> Result<ParsedRequest, RequestError> {
METRICS.put_api_requests.machine_cfg_count.inc();
let config = serde_json::from_slice::<MachineConfig>(body.raw()).map_err(|err| {
let config = serde_json::from_slice::<MachineConfig>(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.machine_cfg_fails.inc();
err
})?;

// Check for the presence of deprecated `cpu_template` field.
Expand All @@ -44,9 +43,8 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result<ParsedRequest, Req
pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, RequestError> {
METRICS.patch_api_requests.machine_cfg_count.inc();
let config_update =
serde_json::from_slice::<MachineConfigUpdate>(body.raw()).map_err(|err| {
serde_json::from_slice::<MachineConfigUpdate>(body.raw()).inspect_err(|_| {
METRICS.patch_api_requests.machine_cfg_fails.inc();
err
})?;

if config_update.is_empty() {
Expand Down
3 changes: 1 addition & 2 deletions src/firecracker/src/api_server/request/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ use super::Body;
pub(crate) fn parse_put_metrics(body: &Body) -> Result<ParsedRequest, RequestError> {
METRICS.put_api_requests.metrics_count.inc();
Ok(ParsedRequest::new_sync(VmmAction::ConfigureMetrics(
serde_json::from_slice::<MetricsConfig>(body.raw()).map_err(|err| {
serde_json::from_slice::<MetricsConfig>(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.metrics_fails.inc();
err
})?,
)))
}
Expand Down
9 changes: 3 additions & 6 deletions src/firecracker/src/api_server/request/mmds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ pub(crate) fn parse_get_mmds() -> Result<ParsedRequest, RequestError> {
}

fn parse_put_mmds_config(body: &Body) -> Result<ParsedRequest, RequestError> {
let config: MmdsConfig = serde_json::from_slice(body.raw()).map_err(|err| {
let config: MmdsConfig = serde_json::from_slice(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.mmds_fails.inc();
err
})?;
// Construct the `ParsedRequest` object.
let version = config.version;
Expand All @@ -42,9 +41,8 @@ pub(crate) fn parse_put_mmds(
METRICS.put_api_requests.mmds_count.inc();
match path_second_token {
None => Ok(ParsedRequest::new_sync(VmmAction::PutMMDS(
serde_json::from_slice(body.raw()).map_err(|err| {
serde_json::from_slice(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.mmds_fails.inc();
err
})?,
))),
Some("config") => parse_put_mmds_config(body),
Expand All @@ -61,9 +59,8 @@ pub(crate) fn parse_put_mmds(
pub(crate) fn parse_patch_mmds(body: &Body) -> Result<ParsedRequest, RequestError> {
METRICS.patch_api_requests.mmds_count.inc();
Ok(ParsedRequest::new_sync(VmmAction::PatchMMDS(
serde_json::from_slice(body.raw()).map_err(|err| {
serde_json::from_slice(body.raw()).inspect_err(|_| {
METRICS.patch_api_requests.mmds_fails.inc();
err
})?,
)))
}
Expand Down
6 changes: 2 additions & 4 deletions src/firecracker/src/api_server/request/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ pub(crate) fn parse_put_net(
return Err(RequestError::EmptyID);
};

let netif = serde_json::from_slice::<NetworkInterfaceConfig>(body.raw()).map_err(|err| {
let netif = serde_json::from_slice::<NetworkInterfaceConfig>(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.network_fails.inc();
err
})?;
if id != netif.iface_id.as_str() {
METRICS.put_api_requests.network_fails.inc();
Expand Down Expand Up @@ -53,9 +52,8 @@ pub(crate) fn parse_patch_net(
};

let netif =
serde_json::from_slice::<NetworkInterfaceUpdateConfig>(body.raw()).map_err(|err| {
serde_json::from_slice::<NetworkInterfaceUpdateConfig>(body.raw()).inspect_err(|_| {
METRICS.patch_api_requests.network_fails.inc();
err
})?;
if id != netif.iface_id {
METRICS.patch_api_requests.network_count.inc();
Expand Down
3 changes: 1 addition & 2 deletions src/firecracker/src/api_server/request/vsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ use super::Body;

pub(crate) fn parse_put_vsock(body: &Body) -> Result<ParsedRequest, RequestError> {
METRICS.put_api_requests.vsock_count.inc();
let vsock_cfg = serde_json::from_slice::<VsockDeviceConfig>(body.raw()).map_err(|err| {
let vsock_cfg = serde_json::from_slice::<VsockDeviceConfig>(body.raw()).inspect_err(|_| {
METRICS.put_api_requests.vsock_fails.inc();
err
})?;

// Check for the presence of deprecated `vsock_id` field.
Expand Down
61 changes: 30 additions & 31 deletions src/vmm/src/arch/aarch64/cache_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@ pub(crate) enum CacheInfoError {
MissingOptionalAttr(String, CacheEntry),
}

struct CacheEngine {
store: Box<dyn CacheStore>,
struct CacheEngine<T: CacheStore = HostCacheStore> {
store: T,
}

trait CacheStore: std::fmt::Debug {
fn get_by_key(&self, index: u8, file_name: &str) -> Result<String, CacheInfoError>;
}

#[derive(Debug)]
pub(crate) struct CacheEntry {
pub struct CacheEntry {
// Cache Level: 1, 2, 3..
pub level: u8,
// Type of cache: Unified, Data, Instruction.
Expand All @@ -45,17 +45,16 @@ pub(crate) struct CacheEntry {
}

#[derive(Debug)]
struct HostCacheStore {
pub struct HostCacheStore {
cache_dir: PathBuf,
}

#[cfg(not(test))]
impl Default for CacheEngine {
impl Default for CacheEngine<HostCacheStore> {
fn default() -> Self {
CacheEngine {
store: Box::new(HostCacheStore {
store: HostCacheStore {
cache_dir: PathBuf::from("/sys/devices/system/cpu/cpu0/cache"),
}),
},
}
}
}
Expand All @@ -72,7 +71,7 @@ impl CacheStore for HostCacheStore {
}

impl CacheEntry {
fn from_index(index: u8, store: &dyn CacheStore) -> Result<CacheEntry, CacheInfoError> {
fn from_index(index: u8, store: &impl CacheStore) -> Result<CacheEntry, CacheInfoError> {
let mut err_str = String::new();
let mut cache: CacheEntry = CacheEntry::default();

Expand Down Expand Up @@ -287,10 +286,10 @@ pub(crate) fn read_cache_config(
// Also without this mechanism we would be logging the warnings for each level which pollutes
// a lot the logs.
let mut logged_missing_attr = false;
let engine = CacheEngine::default();
let engine = CacheEngine::<HostCacheStore>::default();

for index in 0..=MAX_CACHE_LEVEL {
match CacheEntry::from_index(index, engine.store.as_ref()) {
match CacheEntry::from_index(index, &engine.store) {
Ok(cache) => {
append_cache_level(cache_l1, cache_non_l1, cache);
}
Expand Down Expand Up @@ -326,22 +325,22 @@ mod tests {
dummy_fs: HashMap<String, String>,
}

impl Default for CacheEngine {
impl Default for CacheEngine<MockCacheStore> {
fn default() -> Self {
CacheEngine {
store: Box::new(MockCacheStore {
store: MockCacheStore {
dummy_fs: create_default_store(),
}),
},
}
}
}

impl CacheEngine {
impl CacheEngine<MockCacheStore> {
fn new(map: &HashMap<String, String>) -> Self {
CacheEngine {
store: Box::new(MockCacheStore {
store: MockCacheStore {
dummy_fs: map.clone(),
}),
},
}
}
}
Expand Down Expand Up @@ -425,12 +424,12 @@ mod tests {
let mut map1 = default_map.clone();
map1.remove("index0/type");
let engine = CacheEngine::new(&map1);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
// We did create the level file but we still do not have the type file.
assert!(matches!(res.unwrap_err(), CacheInfoError::MissingCacheType));

let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"shared cpu map, coherency line size, size, number of sets",
Expand All @@ -440,15 +439,15 @@ mod tests {
let mut map2 = default_map.clone();
map2.insert("index0/level".to_string(), "d".to_string());
let engine = CacheEngine::new(&map2);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"Invalid cache configuration found for level: invalid digit found in string"
);

default_map.insert("index0/type".to_string(), "Instructionn".to_string());
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"Invalid cache configuration found for type: Instructionn"
Expand All @@ -464,7 +463,7 @@ mod tests {
"00000000,00000001".to_string(),
);
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"coherency line size, size, number of sets"
Expand All @@ -475,15 +474,15 @@ mod tests {
"00000000,0000000G".to_string(),
);
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"Invalid cache configuration found for shared_cpu_map: invalid digit found in string"
);

default_map.insert("index0/shared_cpu_map".to_string(), "00000000".to_string());
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"Invalid cache configuration found for shared_cpu_map: 00000000"
Expand All @@ -496,7 +495,7 @@ mod tests {

default_map.insert("index0/coherency_line_size".to_string(), "64".to_string());
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
"shared cpu map, size, number of sets",
format!("{}", res.unwrap_err())
Expand All @@ -507,7 +506,7 @@ mod tests {
"Instruction".to_string(),
);
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"Invalid cache configuration found for coherency_line_size: invalid digit found in \
Expand All @@ -521,23 +520,23 @@ mod tests {

default_map.insert("index0/size".to_string(), "64K".to_string());
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"shared cpu map, coherency line size, number of sets",
);

default_map.insert("index0/size".to_string(), "64".to_string());
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"Invalid cache configuration found for size: 64"
);

default_map.insert("index0/size".to_string(), "64Z".to_string());
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"Invalid cache configuration found for size: 64Z"
Expand All @@ -550,15 +549,15 @@ mod tests {

default_map.insert("index0/number_of_sets".to_string(), "64".to_string());
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
"shared cpu map, coherency line size, size",
format!("{}", res.unwrap_err())
);

default_map.insert("index0/number_of_sets".to_string(), "64K".to_string());
let engine = CacheEngine::new(&default_map);
let res = CacheEntry::from_index(0, engine.store.as_ref());
let res = CacheEntry::from_index(0, &engine.store);
assert_eq!(
format!("{}", res.unwrap_err()),
"Invalid cache configuration found for number_of_sets: invalid digit found in string"
Expand Down
2 changes: 1 addition & 1 deletion src/vmm/src/devices/legacy/serial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ mod tests {
),
input: None::<std::io::Stdin>,
};
serial.serial.raw_input(&[b'a', b'b', b'c']).unwrap();
serial.serial.raw_input(b"abc").unwrap();

let invalid_reads_before = metrics.missed_read_count.count();
let mut v = [0x00; 2];
Expand Down
2 changes: 1 addition & 1 deletion src/vmm/src/devices/virtio/balloon/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
//!
//! The system implements 1 type of metrics:
//! * Shared Incremental Metrics (SharedIncMetrics) - dedicated for the metrics which need a counter
//! (i.e the number of times an API request failed). These metrics are reset upon flush.
//! (i.e the number of times an API request failed). These metrics are reset upon flush.
use serde::ser::SerializeMap;
use serde::{Serialize, Serializer};
Expand Down
Loading

0 comments on commit bbadfbf

Please sign in to comment.