diff --git a/Cargo.lock b/Cargo.lock index d826332d..cedf6696 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3392,7 +3392,6 @@ dependencies = [ "futures-util", "git-version", "insta", - "lazy_static", "logging_content", "loopdev-3", "mockito", diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000..d5c504a4 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,3 @@ +# Names of products and specifications, which read as prose and take no +# backticks in a doc comment. +doc-valid-idents = ["UpdateHub", "OpenAPI", ".."] diff --git a/updatehub-cloud-sdk/src/api.rs b/updatehub-cloud-sdk/src/api.rs index 8424a4a2..3f711898 100644 --- a/updatehub-cloud-sdk/src/api.rs +++ b/updatehub-cloud-sdk/src/api.rs @@ -53,11 +53,20 @@ impl serde::ser::Serialize for MetadataValue<'_> { } impl UpdatePackage { + /// Parses the raw metadata of an update package. + /// + /// # Errors + /// + /// Returns an error when `content` is not the JSON document the agent + /// expects. pub fn parse(content: &[u8]) -> crate::Result { let update_package = serde_json::from_slice(content)?; Ok(UpdatePackage { inner: update_package, raw: content.to_vec() }) } + /// Returns the SHA-256 sum of the raw metadata, which identifies the + /// package. + #[must_use] pub fn package_uid(&self) -> String { openssl::sha::sha256(&self.raw).iter().fold(String::new(), |mut output, c| { let _ = write!(output, "{c:02x}"); @@ -66,16 +75,30 @@ impl UpdatePackage { }) } + /// Returns the version the package declares. + #[must_use] pub fn version(&self) -> &str { &self.inner.version } } impl Signature { + /// Decodes a signature from its base64 form. + /// + /// # Errors + /// + /// Returns an error when `bytes` is not valid base64. pub fn from_base64_str(bytes: &str) -> crate::Result { - Ok(Signature(openssl::base64::decode_block(bytes)?.to_vec())) + Ok(Signature(openssl::base64::decode_block(bytes)?)) } + /// Checks the signature of `package` against the public key stored at + /// `key`. + /// + /// # Errors + /// + /// Returns an error when the key does not load, when the check itself + /// fails, or when the signature does not match the package. pub fn validate(&self, key: &Path, package: &UpdatePackage) -> crate::Result<()> { use openssl::{hash::MessageDigest, pkey::PKey, rsa::Rsa, sign::Verifier}; let key = PKey::from_rsa(Rsa::public_key_from_pem(&fs::read(key)?)?)?; diff --git a/updatehub-cloud-sdk/src/client.rs b/updatehub-cloud-sdk/src/client.rs index 48c5aa18..11911f2d 100644 --- a/updatehub-cloud-sdk/src/client.rs +++ b/updatehub-cloud-sdk/src/client.rs @@ -16,6 +16,13 @@ pub struct Client<'a> { server: &'a str, } +/// Downloads the content of `url` into `handle`. +/// +/// # Errors +/// +/// Returns an error when `url` does not parse, when the request fails, when the +/// server answers with a status other than success, or when the write to +/// `handle` fails. pub async fn get(url: &str, handle: &mut W) -> Result<()> where W: io::AsyncWrite + Unpin, @@ -35,21 +42,22 @@ where return Err(Error::InvalidStatusResponse(resp.status())); } - let mut written: f32 = 0.; + let mut written: u64 = 0; let mut threshold = 10; let length = match resp.headers().get(header::CONTENT_LENGTH) { - Some(v) => usize::from_str(v.to_str()?)?, + Some(v) => u64::from_str(v.to_str()?)?, None => 0, }; while let Some(chunk) = resp.chunk().await? { - let read = chunk.len(); + let read = chunk.len() as u64; handle.write_all(&chunk).await?; if length > 0 { - written += read as f32 / (length as f32 / 100.); - if written as usize >= threshold { + written += read; + let percent = written * 100 / length; + if percent >= threshold { threshold += 20; - debug!("{}% of the file has been downloaded", std::cmp::min(written as usize, 100)); + debug!("{}% of the file has been downloaded", std::cmp::min(percent, 100)); } } } @@ -59,6 +67,13 @@ where } impl<'a> Client<'a> { + /// Constructs a client that talks to the server at `server`. + /// + /// # Panics + /// + /// Panics when the platform gives no TLS backend to build the HTTP client + /// with. + #[must_use] pub fn new(server: &'a str) -> Self { let mut headers = header::HeaderMap::new(); headers.insert(header::USER_AGENT, header::HeaderValue::from_static("updatehub/2.0 Linux")); @@ -74,9 +89,16 @@ impl<'a> Client<'a> { .build() .unwrap(); - Self { server, client } + Self { client, server } } + /// Asks the server whether an update is available for this device. + /// + /// # Errors + /// + /// Returns an error when the server address does not parse, when the + /// request fails, when the server answers with an unexpected status, or + /// when the update metadata does not parse. pub async fn probe( &self, num_retries: usize, @@ -95,30 +117,40 @@ impl<'a> Client<'a> { match response.status() { StatusCode::NOT_FOUND => Ok(api::ProbeResponse::NoUpdate), StatusCode::OK => { - match response + let extra_poll = response .headers() .get("add-extra-poll") .and_then(|extra_poll| extra_poll.to_str().ok()) - .and_then(|extra_poll| extra_poll.parse().ok()) - { - Some(extra_poll) => Ok(api::ProbeResponse::ExtraPoll(extra_poll)), - None => { - let signature = response - .headers() - .get("UH-Signature") - .map(TryInto::try_into) - .transpose()?; - Ok(api::ProbeResponse::Update( - api::UpdatePackage::parse(&response.bytes().await?)?, - signature, - )) - } + .and_then(|extra_poll| extra_poll.parse().ok()); + + if let Some(extra_poll) = extra_poll { + Ok(api::ProbeResponse::ExtraPoll(extra_poll)) + } else { + let signature = response + .headers() + .get("UH-Signature") + .map(TryInto::try_into) + .transpose()?; + Ok(api::ProbeResponse::Update( + api::UpdatePackage::parse(&response.bytes().await?)?, + signature, + )) } } s => Err(Error::InvalidStatusResponse(s)), } } + /// Downloads one object of an update package into `download_dir`. + /// + /// Downloads that stopped part way continue from the number of bytes + /// already on disk. + /// + /// # Errors + /// + /// Returns an error when the server address does not parse, when the + /// request fails, when the server answers with a status other than + /// success, or when the write to disk fails. pub async fn download_object( &self, product_uid: &str, @@ -152,6 +184,12 @@ impl<'a> Client<'a> { save_body_to(request.send().await?, &mut file).await } + /// Reports the current state of the device to the server. + /// + /// # Errors + /// + /// Returns an error when the server address does not parse or when the + /// request fails. pub async fn report( &self, state: &str, @@ -161,8 +199,6 @@ impl<'a> Client<'a> { error_message: Option, current_log: Option, ) -> Result<()> { - validate_url(self.server)?; - #[derive(serde::Serialize)] #[serde(rename_all = "kebab-case")] struct Payload<'a> { @@ -179,6 +215,8 @@ impl<'a> Client<'a> { current_log: Option, } + validate_url(self.server)?; + let payload = Payload { state, firmware, package_uid, previous_state, error_message, current_log }; diff --git a/updatehub-package-schema/src/definitions/chunk_size.rs b/updatehub-package-schema/src/definitions/chunk_size.rs index 016a7a08..c590cb7d 100644 --- a/updatehub-package-schema/src/definitions/chunk_size.rs +++ b/updatehub-package-schema/src/definitions/chunk_size.rs @@ -46,7 +46,7 @@ mod test { serde_json::from_value::(json!({ "chunk_size": 313 })).ok(), Some(Payload { chunk_size: ChunkSize(313) }) ); - assert!(serde_json::from_value::(json!({ "chunk_size": 0 })).is_err()) + assert!(serde_json::from_value::(json!({ "chunk_size": 0 })).is_err()); } #[test] diff --git a/updatehub-package-schema/src/definitions/install_if_different.rs b/updatehub-package-schema/src/definitions/install_if_different.rs index da4b49c0..fe402115 100644 --- a/updatehub-package-schema/src/definitions/install_if_different.rs +++ b/updatehub-package-schema/src/definitions/install_if_different.rs @@ -91,6 +91,6 @@ mod test { "pattern": "linux-kernel" })) .unwrap() - ) + ); } } diff --git a/updatehub-sdk/src/api/info/firmware.rs b/updatehub-sdk/src/api/info/firmware.rs index 9143a556..6e6f1130 100644 --- a/updatehub-sdk/src/api/info/firmware.rs +++ b/updatehub-sdk/src/api/info/firmware.rs @@ -51,10 +51,12 @@ impl MetadataValue { self.0.keys() } + #[must_use] pub fn is_empty(&self) -> bool { self.0.len() == 0 } + #[must_use] pub fn len(&self) -> usize { self.0.len() } diff --git a/updatehub-sdk/src/client.rs b/updatehub-sdk/src/client.rs index 2940dd39..92bf2274 100644 --- a/updatehub-sdk/src/client.rs +++ b/updatehub-sdk/src/client.rs @@ -24,6 +24,7 @@ impl Default for Client { impl Client { /// Constructs a new `Client`. + #[must_use] pub fn new(server_address: &str) -> Self { Client { server_address: format!("http://{server_address}"), ..Self::default() } } diff --git a/updatehub-sdk/src/lib.rs b/updatehub-sdk/src/lib.rs index 3dcebcf2..af34d1c5 100644 --- a/updatehub-sdk/src/lib.rs +++ b/updatehub-sdk/src/lib.rs @@ -7,12 +7,12 @@ //! When running an agent instance, the API provides some methods //! for communicating with UpdateHub: //! -//! - [abort_download](Client::abort_download) +//! - [`abort_download`](Client::abort_download) //! - [info](Client::info) -//! - [local_install](Client::local_install) +//! - [`local_install`](Client::local_install) //! - [log](Client::log) //! - [probe](Client::probe) -//! - [remote_install](Client::remote_install) +//! - [`remote_install`](Client::remote_install) pub mod api; mod client; diff --git a/updatehub-sdk/src/listener.rs b/updatehub-sdk/src/listener.rs index 66c26206..64afcad3 100644 --- a/updatehub-sdk/src/listener.rs +++ b/updatehub-sdk/src/listener.rs @@ -65,11 +65,21 @@ pub struct Handler { impl Handler { /// Cancels the current action on the agent. + /// + /// # Errors + /// + /// Returns an error when the write to the agent socket fails. pub async fn cancel(&mut self) -> Result<()> { self.stream.lock().await.write_all(b"cancel").await.map_err(Error::Io) } /// Tell the agent to proceed with the transition. + /// + /// # Errors + /// + /// Never returns an error. The agent proceeds when it receives no message + /// at all, so this function only keeps the shape of the other handler + /// commands. pub async fn proceed(&self) -> Result<()> { // No message need to be sent to the connection in order to the // agent to proceed handling the current state. @@ -80,6 +90,7 @@ impl Handler { impl StateChange { /// Creates a new `StateChange` struct. #[inline] + #[must_use] pub fn new() -> Self { StateChange::default() } @@ -103,10 +114,15 @@ impl StateChange { F: Fn(Handler) -> Fut + 'static, Fut: Future> + 'static, { - self.callbacks.entry(state).or_default().push(Box::new(move |d| Box::pin(f(d)))) + self.callbacks.entry(state).or_default().push(Box::new(move |d| Box::pin(f(d)))); } /// Start the agent to listen for messages on the socket. + /// + /// # Errors + /// + /// Returns an error when the socket cannot be created or read, or when a + /// registered callback returns an error. pub async fn listen(&self) -> Result<()> { let sdk_trigger = Path::new(SDK_TRIGGER_FILENAME); if !sdk_trigger.exists() { diff --git a/updatehub-sdk/tests/openapi_integration.rs b/updatehub-sdk/tests/openapi_integration.rs index 365d2a68..da7bfe8e 100644 --- a/updatehub-sdk/tests/openapi_integration.rs +++ b/updatehub-sdk/tests/openapi_integration.rs @@ -43,8 +43,7 @@ async fn probe_default() { let client = sdk::Client::new(&addr); let response = client.probe(None).await; match dbg!(response) { - Ok(_) => {} - Err(sdk::Error::AgentIsBusy(_)) => {} + Ok(_) | Err(sdk::Error::AgentIsBusy(_)) => {} Err(e) => panic!("Unexpected Error response: {e}"), } } @@ -55,8 +54,7 @@ async fn probe_custom() { let client = sdk::Client::new(&addr); let response = client.probe(Some(String::from("http://foo.bar"))).await; match dbg!(response) { - Ok(_) => {} - Err(sdk::Error::AgentIsBusy(_)) => {} + Ok(_) | Err(sdk::Error::AgentIsBusy(_)) => {} Err(e) => panic!("Unexpected Error response: {e}"), } } @@ -69,8 +67,7 @@ async fn local_install() { let response = client.local_install(file.path()).await; match dbg!(response) { - Ok(_) => {} - Err(sdk::Error::AgentIsBusy(_)) => {} + Ok(_) | Err(sdk::Error::AgentIsBusy(_)) => {} Err(e) => panic!("Unexpected Error response: {e}"), } } @@ -81,8 +78,7 @@ async fn remote_install() { let client = sdk::Client::new(&addr); let response = client.remote_install("http://foo.bar").await; match dbg!(response) { - Ok(_) => {} - Err(sdk::Error::AgentIsBusy(_)) => {} + Ok(_) | Err(sdk::Error::AgentIsBusy(_)) => {} Err(e) => panic!("Unexpected Error response: {e}"), } } @@ -93,8 +89,7 @@ async fn abort_download() { let client = sdk::Client::new(&addr); let response = client.abort_download().await; match dbg!(response) { - Ok(_) => {} - Err(sdk::Error::AbortDownloadRefused(_)) => {} + Ok(_) | Err(sdk::Error::AbortDownloadRefused(_)) => {} Err(e) => panic!("Unexpected Error response: {e}"), } } diff --git a/updatehub/Cargo.toml b/updatehub/Cargo.toml index 8548d9a3..06083535 100644 --- a/updatehub/Cargo.toml +++ b/updatehub/Cargo.toml @@ -71,7 +71,6 @@ derive_more = { version = "2", default-features = false, features = [ easy_process = "0.2" find-binary-version = "0.5" futures-util = { version = "0.3", default-features = false } -lazy_static = "1" logging_content = "0.1" mockito = { version = "1", optional = true } ms-converter = "1" diff --git a/updatehub/src/build_info.rs b/updatehub/src/build_info.rs index 80a45961..0537b16c 100644 --- a/updatehub/src/build_info.rs +++ b/updatehub/src/build_info.rs @@ -14,6 +14,7 @@ /// /// println!("Running version: {}", updatehub::version()); /// ``` +#[must_use] pub fn version() -> &'static str { env!("VERSION") } diff --git a/updatehub/src/cloud_mock.rs b/updatehub/src/cloud_mock.rs index 8f738f2f..9787b723 100644 --- a/updatehub/src/cloud_mock.rs +++ b/updatehub/src/cloud_mock.rs @@ -64,7 +64,7 @@ impl<'a> Client<'a> { object: &str, ) -> Result<()> { if let Some(data) = OBJECT_DATA.with(|conf| conf.borrow_mut().take()) { - tokio::fs::write(download_dir.join(object), data).await? + tokio::fs::write(download_dir.join(object), data).await?; } Ok(()) diff --git a/updatehub/src/firmware/hook.rs b/updatehub/src/firmware/hook.rs index 82de6648..d4388dbc 100644 --- a/updatehub/src/firmware/hook.rs +++ b/updatehub/src/firmware/hook.rs @@ -10,7 +10,7 @@ use walkdir::WalkDir; pub(crate) fn run_hook(path: &Path) -> Result { if !path.exists() { - return Ok("".into()); + return Ok(String::new()); } run_script(path.to_str().expect("invalid path for hook")) @@ -35,7 +35,7 @@ pub(crate) fn run_script(cmd: &str) -> Result { Err(easy_process::Error::Failure(status, output)) => { error!("Script {} failed to run: {}", cmd, status); if !output.stderr.is_empty() { - output.stderr.lines().for_each(|err| error!("{} (stderr): {}", cmd, err)) + output.stderr.lines().for_each(|err| error!("{} (stderr): {}", cmd, err)); } Err(easy_process::Error::Failure(status, output).into()) } diff --git a/updatehub/src/firmware/mod.rs b/updatehub/src/firmware/mod.rs index 625827df..86e1d256 100644 --- a/updatehub/src/firmware/mod.rs +++ b/updatehub/src/firmware/mod.rs @@ -57,6 +57,12 @@ pub(crate) enum Transition { pub struct Metadata(pub api::Metadata); impl Metadata { + /// Collects the firmware metadata by running the hooks stored under `path`. + /// + /// # Errors + /// + /// Returns an error when a hook is missing, when a hook fails, or when the + /// output of a hook does not have the expected form. pub fn from_path(path: &Path) -> Result { let product_uid_hook = path.join(PRODUCT_UID_HOOK); let version_hook = path.join(VERSION_HOOK); diff --git a/updatehub/src/http_api.rs b/updatehub/src/http_api.rs index 349c4065..0d334be6 100644 --- a/updatehub/src/http_api.rs +++ b/updatehub/src/http_api.rs @@ -13,7 +13,7 @@ pub(crate) struct Api(); impl Api { pub(crate) async fn serve(addr: machine::Addr, socket: std::net::SocketAddr) { - warp::serve(Api::filter(addr)).run(socket).await + warp::serve(Api::filter(addr)).run(socket).await; } fn filter(addr: machine::Addr) -> warp::filters::BoxedFilter<(impl warp::Reply,)> { diff --git a/updatehub/src/logger.rs b/updatehub/src/logger.rs index 7de5d282..f4760682 100644 --- a/updatehub/src/logger.rs +++ b/updatehub/src/logger.rs @@ -3,13 +3,11 @@ // SPDX-License-Identifier: Apache-2.0 use crate::mem_drain::MemDrain; -use lazy_static::lazy_static; use slog::{Drain, Logger, o}; -use std::sync::{Arc, Mutex, MutexGuard}; +use std::sync::{Arc, LazyLock, Mutex, MutexGuard}; -lazy_static! { - static ref BUFFER: Arc> = Arc::new(Mutex::new(MemDrain::default())); -} +static BUFFER: LazyLock>> = + LazyLock::new(|| Arc::new(Mutex::new(MemDrain::default()))); pub fn init(level: slog::Level) -> slog_scope::GlobalLoggerGuard { let buffer_drain = buffer().filter_level(level).fuse(); @@ -33,15 +31,15 @@ pub fn buffer() -> Arc> { /// The buffered drain, recovering the lock if a thread panicked while holding /// it: being unable to log must not bring the agent down. fn buffer_lock() -> MutexGuard<'static, MemDrain> { - BUFFER.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) + BUFFER.lock().unwrap_or_else(std::sync::PoisonError::into_inner) } pub fn start_memory_logging() { - buffer_lock().start_logging() + buffer_lock().start_logging(); } pub fn stop_memory_logging() { - buffer_lock().stop_logging() + buffer_lock().stop_logging(); } /// Record what `f` logs even while memory logging is stopped. @@ -61,6 +59,9 @@ pub fn record_out_of_scope(f: impl FnOnce() -> R) -> R { result } +/// Returns everything the memory drain holds for the current scope of +/// operation. +#[must_use] pub fn get_memory_log() -> String { buffer_lock().to_string() } diff --git a/updatehub/src/main.rs b/updatehub/src/main.rs index ef836533..2ad433f2 100644 --- a/updatehub/src/main.rs +++ b/updatehub/src/main.rs @@ -97,7 +97,7 @@ struct DaemonOptions { fn verbosity_level(value: &str) -> Result { use std::str::FromStr; - slog::Level::from_str(value).map_err(|_| format!("failed to parse verbosity level: {value}")) + slog::Level::from_str(value).map_err(|()| format!("failed to parse verbosity level: {value}")) } async fn daemon_main(cmd: DaemonOptions) -> updatehub::Result<()> { @@ -157,10 +157,10 @@ async fn client_main(client_options: ClientOptions) -> updatehub::Result<()> { } else { match response { sdk::api::probe::Response::Updating => { - println!("Update available. The update is running in background.") + println!("Update available. The update is running in background."); } sdk::api::probe::Response::NoUpdate => { - println!("There are no updates available.") + println!("There are no updates available."); } sdk::api::probe::Response::TryAgain(t) => { println!("Server replied asking us to try again in {t} seconds"); diff --git a/updatehub/src/mem_drain.rs b/updatehub/src/mem_drain.rs index 04c001b8..0761b74b 100644 --- a/updatehub/src/mem_drain.rs +++ b/updatehub/src/mem_drain.rs @@ -128,11 +128,11 @@ impl MemDrain { /// The recorded entries, recovering the lock if a thread panicked while /// holding it: being unable to log must not bring the agent down. fn records(&self) -> RwLockReadGuard<'_, Records> { - self.records.read().unwrap_or_else(|poisoned| poisoned.into_inner()) + self.records.read().unwrap_or_else(std::sync::PoisonError::into_inner) } fn records_mut(&self) -> RwLockWriteGuard<'_, Records> { - self.records.write().unwrap_or_else(|poisoned| poisoned.into_inner()) + self.records.write().unwrap_or_else(std::sync::PoisonError::into_inner) } /// Start recording a new operation, discarding what the previous one left. diff --git a/updatehub/src/object/installer/copy.rs b/updatehub/src/object/installer/copy.rs index d4ea6690..01db42b2 100644 --- a/updatehub/src/object/installer/copy.rs +++ b/updatehub/src/object/installer/copy.rs @@ -45,7 +45,7 @@ impl Installer for objects::Copy { let device = self.target_type.get_target().log_error_msg("failed to get target device")?; let filesystem = self.filesystem; let mount_options = &self.mount_options; - let format_options = &self.target_format.format_options; + let format_options = self.target_format.format_options.as_deref(); let chunk_size = definitions::ChunkSize::default().0; let sha256sum = self.sha256sum(); let target_path = self.target_path.strip_prefix("/").unwrap_or(&self.target_path); @@ -56,7 +56,7 @@ impl Installer for objects::Copy { let file_path = mount_guard.mount_point().join(target_path); let should_skip_install = file_path.exists() && super::should_skip_install( - &self.install_if_different, + self.install_if_different.as_ref(), &self.sha256sum, async move { Ok(fs::File::open(file_path).await?) }, ) @@ -113,8 +113,8 @@ impl Installer for objects::Copy { utils::fs::chown( &dest, - &self.target_permissions.target_uid, - &self.target_permissions.target_gid, + self.target_permissions.target_uid.as_ref(), + self.target_permissions.target_gid.as_ref(), ) .log_error_msg("failed to update ownership")?; @@ -164,7 +164,7 @@ mod tests { }; // Format the faked device - utils::fs::format(&device, definitions::Filesystem::Ext4, &None)?; + utils::fs::format(&device, definitions::Filesystem::Ext4, None)?; // Generate the source file let download_dir = tempfile::tempdir()?; @@ -193,12 +193,12 @@ mod tests { utils::fs::chmod(&file, mode)?; } - utils::fs::chown(&file, &perm.target_uid, &perm.target_gid)?; + utils::fs::chown(&file, perm.target_uid.as_ref(), perm.target_gid.as_ref())?; } // Generate base copy object let mut obj = objects::Copy { - filename: "".to_string(), + filename: String::new(), filesystem: definitions::Filesystem::Ext4, size: FILE_SIZE as u64, sha256sum: source.path().to_string_lossy().to_string(), @@ -245,17 +245,17 @@ mod tests { let metadata = dest.metadata()?; if let Some(mode) = obj.target_permissions.target_mode { assert_eq!(mode, metadata.mode() % 0o1000); - }; + } if let Some(uid) = obj.target_permissions.target_uid { let uid = uid.as_u32(); assert_eq!(uid, metadata.uid()); - }; + } if let Some(gid) = obj.target_permissions.target_gid { let gid = gid.as_u32(); assert_eq!(gid, metadata.gid()); - }; + } } loopdev.detach()?; @@ -264,13 +264,13 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "needs root to attach a loop device"] async fn copy_compressed_file() { exec_test_with_copy(|obj| obj.compressed = true, None, true).await.unwrap(); } #[tokio::test] - #[ignore] + #[ignore = "needs root to attach a loop device"] async fn copy_over_formated_partion() { exec_test_with_copy(|obj| obj.target_format.should_format = true, None, false) .await @@ -278,7 +278,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "needs root to attach a loop device"] async fn copy_over_existing_file() { exec_test_with_copy( |_| (), @@ -294,12 +294,12 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "needs root to attach a loop device"] async fn copy_change_uid() { exec_test_with_copy( |obj| { obj.target_permissions.target_uid = - Some(definitions::target_permissions::Uid::Number(0)) + Some(definitions::target_permissions::Uid::Number(0)); }, None, false, @@ -309,12 +309,12 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "needs root to attach a loop device"] async fn copy_change_gid() { exec_test_with_copy( |obj| { obj.target_permissions.target_gid = - Some(definitions::target_permissions::Gid::Number(0)) + Some(definitions::target_permissions::Gid::Number(0)); }, Some(definitions::TargetPermissions { target_mode: Some(0o666), @@ -328,7 +328,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "needs root to attach a loop device"] async fn copy_change_mode() { exec_test_with_copy( |obj| obj.target_permissions.target_mode = Some(0o444), diff --git a/updatehub/src/object/installer/flash.rs b/updatehub/src/object/installer/flash.rs index 6a141c49..9fb0be95 100644 --- a/updatehub/src/object/installer/flash.rs +++ b/updatehub/src/object/installer/flash.rs @@ -30,7 +30,9 @@ impl Installer for objects::Flash { )?; Ok(()) } - _ => Err(Error::InvalidTargetType(self.target.clone())), + definitions::TargetType::UBIVolume(_) => { + Err(Error::InvalidTargetType(self.target.clone())) + } } } @@ -40,7 +42,7 @@ impl Installer for objects::Flash { let target = self.target.get_target()?; let source = context.download_dir.join(self.sha256sum()); - if super::should_skip_install(&self.install_if_different, &self.sha256sum, async { + if super::should_skip_install(self.install_if_different.as_ref(), &self.sha256sum, async { tokio::fs::File::open(&target).await.map_err(Error::from) }) .await? @@ -123,7 +125,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "needs root to load the MTD simulator modules"] async fn install_nor() { let _mtd_lock = SERIALIZE.lock(); let mtd = FakeMtd::new(&["system0"], MtdKind::Nor).unwrap(); diff --git a/updatehub/src/object/installer/imxkobs.rs b/updatehub/src/object/installer/imxkobs.rs index 15b47cce..fc853280 100644 --- a/updatehub/src/object/installer/imxkobs.rs +++ b/updatehub/src/object/installer/imxkobs.rs @@ -36,15 +36,18 @@ impl Installer for objects::Imxkobs { async fn install(&self, context: &Context) -> Result<()> { info!("'imxkobs' handler Install {} ({})", self.filename, self.sha256sum); - let should_skip_install = - super::should_skip_install(&self.install_if_different, &self.sha256sum, async { + let should_skip_install = super::should_skip_install( + self.install_if_different.as_ref(), + &self.sha256sum, + async { let path = chip_0_path(self); let f = path.file_name().ok_or(Error::InvalidPath)?; let mut file_name = f.to_os_string(); file_name.push("ro"); tokio::fs::File::open(path.with_file_name(file_name)).await.map_err(Error::from) - }) - .await?; + }, + ) + .await?; if should_skip_install { return Ok(()); } @@ -52,8 +55,8 @@ impl Installer for objects::Imxkobs { let mut cmd = String::from("kobs-ng init "); if self.padding_1k { - cmd += "-x " - }; + cmd += "-x "; + } cmd += context .download_dir diff --git a/updatehub/src/object/installer/mod.rs b/updatehub/src/object/installer/mod.rs index 517fdafd..ee8bfa87 100644 --- a/updatehub/src/object/installer/mod.rs +++ b/updatehub/src/object/installer/mod.rs @@ -76,7 +76,8 @@ async fn check_if_different( } definitions::InstallIfDifferent::CustomPattern { version, pattern } => { handle.seek(io::SeekFrom::Start(pattern.seek)).await?; - let mut src = BufReader::with_capacity(pattern.buffer_size as usize, handle); + let buffer_size = usize::try_from(pattern.buffer_size).unwrap_or(usize::MAX); + let mut src = BufReader::with_capacity(buffer_size, handle); if let Some(ref cur_version) = fbv::version_with_pattern(&mut src, &pattern.regexp).await { @@ -90,7 +91,7 @@ async fn check_if_different( } async fn should_skip_install( - rule: &Option, + rule: Option<&definitions::InstallIfDifferent>, sha256sum: &str, handler: F, ) -> Result @@ -129,20 +130,17 @@ where #[cfg(test)] pub(crate) mod tests { use super::*; - use lazy_static::lazy_static; use std::{ fs, io::Write, os::unix::fs::PermissionsExt, path::{Path, PathBuf}, - sync::{Arc, Mutex}, + sync::{Arc, LazyLock, Mutex}, }; use tempfile::TempDir; // Used to serialize access to Loop devices across tests - lazy_static! { - pub static ref SERIALIZE: Arc> = Arc::new(Mutex::default()); - } + pub static SERIALIZE: LazyLock>> = LazyLock::new(|| Arc::new(Mutex::default())); fn create_echo_bin(bin: &Path, output: &Path) -> std::io::Result<()> { let mut file = std::fs::File::create(bin)?; diff --git a/updatehub/src/object/installer/raw.rs b/updatehub/src/object/installer/raw.rs index bd8c6946..af5c5ffb 100644 --- a/updatehub/src/object/installer/raw.rs +++ b/updatehub/src/object/installer/raw.rs @@ -38,9 +38,8 @@ impl Installer for objects::Raw { async fn install(&self, context: &Context) -> Result<()> { info!("'raw' handler Install {} ({})", self.filename, self.sha256sum); - let device = match self.target_type { - definitions::TargetType::Device(ref p) => p, - _ => unreachable!("device should be secured by check_requirements"), + let definitions::TargetType::Device(ref device) = self.target_type else { + unreachable!("device should be secured by check_requirements") }; let source = context.download_dir.join(self.sha256sum()); let chunk_size = self.chunk_size.0; @@ -49,8 +48,10 @@ impl Installer for objects::Raw { let truncate = self.truncate.0; let count = self.count.clone(); - let should_skip_install = - super::should_skip_install(&self.install_if_different, &self.sha256sum, async { + let should_skip_install = super::should_skip_install( + self.install_if_different.as_ref(), + &self.sha256sum, + async { trait AsyncReadSeek: AsyncRead + AsyncSeek + Unpin {} impl AsyncReadSeek for R {} @@ -60,12 +61,14 @@ impl Installer for objects::Raw { let h: Box = match &count { definitions::Count::All => Box::new(h), definitions::Count::Limited(n) => { - Box::new(h.take_with_seek((*n as usize * chunk_size) as u64)) + let count = u64::try_from(*n).unwrap_or(0); + Box::new(h.take_with_seek(count * chunk_size as u64)) } }; Ok(h) - }) - .await?; + }, + ) + .await?; if should_skip_install { return Ok(()); } @@ -79,7 +82,8 @@ impl Installer for objects::Raw { match count { definitions::Count::All => Box::new(input), definitions::Count::Limited(n) => { - Box::new(input.take((n as usize * chunk_size) as u64)) + let count = u64::try_from(n).unwrap_or(0); + Box::new(input.take(count * chunk_size as u64)) } } }; @@ -135,7 +139,8 @@ mod tests { let download_dir = tempdir()?; let mut source = NamedTempFile::new_in(download_dir.path())?; - let original_data = std::iter::repeat_n(ORIGINAL_BYTE, size as usize).collect::>(); + let data_len = usize::try_from(size).unwrap(); + let original_data = std::iter::repeat_n(ORIGINAL_BYTE, data_len).collect::>(); let data = if compressed { let mut e = GzEncoder::new(Vec::new(), Compression::default()); e.write_all(&original_data).unwrap(); @@ -147,12 +152,12 @@ mod tests { source.seek(SeekFrom::Start(0))?; let mut dest = NamedTempFile::new_in(download_dir.path())?; - dest.write_all(&std::iter::repeat_n(DEFAULT_BYTE, size as usize).collect::>())?; + dest.write_all(&std::iter::repeat_n(DEFAULT_BYTE, data_len).collect::>())?; dest.seek(SeekFrom::Start(0))?; Ok(( objects::Raw { - filename: "".to_string(), + filename: String::new(), size, sha256sum: source.path().to_string_lossy().to_string(), target_type: definitions::TargetType::Device(dest.path().into()), @@ -200,7 +205,7 @@ mod tests { seek: u64, count: definitions::Count, ) -> io::Result<()> { - let skip = skip as usize * chunk_size; + let skip = usize::try_from(skip).unwrap() * chunk_size; let file = fs::File::open(file).await?; let mut f1 = io::BufReader::with_capacity(chunk_size, &data[skip..]); let mut f2 = io::BufReader::with_capacity(chunk_size, file); diff --git a/updatehub/src/object/installer/tarball.rs b/updatehub/src/object/installer/tarball.rs index 5a3cb4a1..aa378497 100644 --- a/updatehub/src/object/installer/tarball.rs +++ b/updatehub/src/object/installer/tarball.rs @@ -41,7 +41,7 @@ impl Installer for objects::Tarball { let device = self.target.get_target().log_error_msg("failed to get target device")?; let filesystem = self.filesystem; let mount_options = &self.mount_options; - let format_options = &self.target_format.format_options; + let format_options = self.target_format.format_options.as_deref(); let sha256sum = self.sha256sum(); let target_path = self.target_path.strip_prefix("/").unwrap_or(&self.target_path); let source = context.download_dir.join(sha256sum); @@ -100,11 +100,11 @@ mod tests { }; // Format the faked device - utils::fs::format(&device, definitions::Filesystem::Ext4, &None)?; + utils::fs::format(&device, definitions::Filesystem::Ext4, None)?; // Generate base copy object let mut obj = objects::Tarball { - filename: "".to_string(), + filename: String::new(), filesystem: definitions::Filesystem::Ext4, size: CONTENT_SIZE as u64, sha256sum: "tree.tar".to_string(), @@ -153,13 +153,13 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "needs root to attach a loop device"] async fn install_over_formated_partion() { exec_test_with_tarball(|obj| obj.target_format.should_format = true).await.unwrap(); } #[tokio::test] - #[ignore] + #[ignore = "needs root to attach a loop device"] async fn install_over_unformated_partion() { exec_test_with_tarball(|obj| obj.target_path = PathBuf::from("/existing_dir")) .await diff --git a/updatehub/src/object/installer/ubifs.rs b/updatehub/src/object/installer/ubifs.rs index 2a229d8b..4ef7efe8 100644 --- a/updatehub/src/object/installer/ubifs.rs +++ b/updatehub/src/object/installer/ubifs.rs @@ -105,7 +105,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "needs root to load the MTD simulator modules"] async fn install() { let _mtd_lock = SERIALIZE.lock(); let _ubi = FakeUbi::new(&["home"], MtdKind::Nor).unwrap(); diff --git a/updatehub/src/runtime_settings.rs b/updatehub/src/runtime_settings.rs index 8cf9d780..2297f261 100644 --- a/updatehub/src/runtime_settings.rs +++ b/updatehub/src/runtime_settings.rs @@ -63,6 +63,19 @@ impl Default for RuntimeSettings { } impl RuntimeSettings { + /// Loads the runtime settings stored at `path`, or the default ones when + /// the file is absent. + /// + /// A file that fails to parse is renamed with a `.old` suffix, and the + /// default settings take its place. + /// + /// # Errors + /// + /// Returns an error when the file exists but cannot be read. + /// + /// # Panics + /// + /// Panics when `path` names no file, or when that name is not valid UTF-8. pub fn load(path: &Path) -> Result { let mut this = if path.exists() { debug!("loading runtime settings from {:?}", path); diff --git a/updatehub/src/settings.rs b/updatehub/src/settings.rs index 6ec2a34e..82f1816a 100644 --- a/updatehub/src/settings.rs +++ b/updatehub/src/settings.rs @@ -59,6 +59,11 @@ impl Settings { /// Loads the settings from the filesystem. If /// `/etc/updatehub.conf` does not exists, it uses the default /// settings. + /// + /// # Errors + /// + /// Returns an error when the file cannot be read, when it does not parse, + /// or when a value it holds fails validation. pub fn load(path: &Path) -> Result { if path.exists() { debug!("loading system settings from {:?}", path); @@ -172,7 +177,7 @@ fn v1_parse(content: &str, toml_err: toml::de::Error) -> Result { "dry-run", "copy", "flash", "imxkobs", "raw", "tarball", "ubifs", ] .iter() - .map(|i| i.to_string()) + .map(|i| (*i).to_string()) .collect(), } } @@ -340,7 +345,7 @@ metadata="/usr/share/updatehub" "dry-run", "copy", "flash", "imxkobs", "raw", "tarball", "ubifs", ] .iter() - .map(|i| i.to_string()) + .map(|i| (*i).to_string()) .collect(), }, network: api::Network { @@ -381,7 +386,10 @@ ListenSocket=localhost:8313 }, update: api::Update { download_dir: "/tmp/download".into(), - supported_install_modes: ["mode1", "mode2"].iter().map(|i| i.to_string()).collect(), + supported_install_modes: ["mode1", "mode2"] + .iter() + .map(|i| (*i).to_string()) + .collect(), }, network: api::Network { server_address: "http://localhost".to_string(), diff --git a/updatehub/src/states/direct_download.rs b/updatehub/src/states/direct_download.rs index 8fff1bc1..61dbac82 100644 --- a/updatehub/src/states/direct_download.rs +++ b/updatehub/src/states/direct_download.rs @@ -28,7 +28,7 @@ impl StateChangeImpl for DirectDownload { async fn handle(self, context: &mut Context) -> Result<(State, machine::StepTransition)> { info!("fetching update package directly from url: {:?}", self.url); - use std::ops::DerefMut; + let communication_receiver = &context.communication.receiver.clone(); let context = Mutex::new(context); @@ -48,9 +48,8 @@ impl StateChangeImpl for DirectDownload { let message_handle_future = async { while let Ok((msg, responder)) = communication_receiver.recv().await { - if let Some(new_state) = self - .handle_communication(msg, responder, context.lock().await.deref_mut()) - .await + if let Some(new_state) = + self.handle_communication(msg, responder, *context.lock().await).await { return Ok(new_state); } diff --git a/updatehub/src/states/download.rs b/updatehub/src/states/download.rs index 555fd135..a8950490 100644 --- a/updatehub/src/states/download.rs +++ b/updatehub/src/states/download.rs @@ -28,7 +28,7 @@ impl Download { ) -> Result<()> { let installation_set = installation_set::inactive().log_error_msg("unable to get current installation set")?; - let download_dir = context.lock().await.settings.update.download_dir.to_owned(); + let download_dir = context.lock().await.settings.update.download_dir.clone(); update_package .clear_unrelated_files(&download_dir, installation_set, &context.lock().await.settings) @@ -58,11 +58,15 @@ impl Download { Some((filename, sha256sum)) } - (filename, sha256sum, Ok(object::info::Status::Missing)) - | (filename, sha256sum, Ok(object::info::Status::Incomplete)) - | (filename, sha256sum, Ok(object::info::Status::Corrupted)) => { - Some((filename, sha256sum)) - } + ( + filename, + sha256sum, + Ok( + object::info::Status::Missing + | object::info::Status::Incomplete + | object::info::Status::Corrupted, + ), + ) => Some((filename, sha256sum)), (_, _, Ok(object::info::Status::Ready)) => None, } @@ -125,7 +129,6 @@ impl StateChangeImpl for Download { } async fn handle(self, context: &mut Context) -> Result<(State, machine::StepTransition)> { - use std::ops::DerefMut; let communication_receiver = &context.communication.receiver.clone(); let context = Mutex::new(context); @@ -137,9 +140,8 @@ impl StateChangeImpl for Download { let message_handle_future = async { while let Ok((msg, responder)) = communication_receiver.recv().await { - if let Some(new_state) = self - .handle_communication(msg, responder, context.lock().await.deref_mut()) - .await + if let Some(new_state) = + self.handle_communication(msg, responder, *context.lock().await).await { return Ok(Some(new_state)); } @@ -229,14 +231,12 @@ mod test { } #[tokio::test] - #[ignore] async fn download_small_object() { - test_object_download(16).await + test_object_download(16).await; } #[tokio::test] - #[ignore] async fn download_large_object() { - test_object_download(100_000_000).await + test_object_download(100_000_000).await; } } diff --git a/updatehub/src/states/mod.rs b/updatehub/src/states/mod.rs index 3340275c..80c06538 100644 --- a/updatehub/src/states/mod.rs +++ b/updatehub/src/states/mod.rs @@ -165,7 +165,7 @@ trait ProgressReporter: CallbackReporter { Ok((state, trans)) => { if let Err(e) = report(leave_state, None, None, None).await { warn!("report failed: {}", e); - }; + } Ok((state, trans)) } Err(e) => { @@ -348,6 +348,12 @@ impl State { /// # Ok(()) /// # } /// ``` +/// +/// # Errors +/// +/// Returns an error when the settings or the runtime settings fail to load, +/// when the firmware metadata cannot be collected, or when the agent cannot +/// listen on its socket. pub async fn run(settings: &Path) -> crate::Result<()> { crate::logger::start_memory_logging(); let settings = Settings::load(settings)?; diff --git a/updatehub/src/states/probe.rs b/updatehub/src/states/probe.rs index a45f3ad7..b3f2827a 100644 --- a/updatehub/src/states/probe.rs +++ b/updatehub/src/states/probe.rs @@ -94,7 +94,7 @@ impl StateChangeImpl for Probe { match probe { ProbeResponse::NoUpdate => { crate::logger::record_out_of_scope(|| { - info!("no update is current available for this device") + info!("no update is current available for this device"); }); // Store timestamp of last polling @@ -107,7 +107,7 @@ impl StateChangeImpl for Probe { ProbeResponse::ExtraPoll(s) => { crate::logger::record_out_of_scope(|| { - info!("delaying the probing for {} seconds as requested by the server", s) + info!("delaying the probing for {} seconds as requested by the server", s); }); Ok((State::Probe(self), machine::StepTransition::Delayed(Duration::seconds(s)))) } diff --git a/updatehub/src/states/tests.rs b/updatehub/src/states/tests.rs index b43ce99a..ba111e94 100644 --- a/updatehub/src/states/tests.rs +++ b/updatehub/src/states/tests.rs @@ -92,7 +92,7 @@ fn validate_v1_restored_runtime_settings() { ) .unwrap(); // Overwrite runtimesettings with a v1 model - let original_runtime_settings = r#" + let original_runtime_settings = r" [Polling] LastPoll=2021-06-01T14:38:57-03:00 FirstPoll=2021-05-01T13:33:33-03:00 @@ -102,7 +102,7 @@ ProbeASAP=false [Update] UpgradeToInstallation=0 -"#; +"; std::fs::write(&setup.runtime_settings.stored_path, original_runtime_settings).unwrap(); let mut loaded_runtime_settings = RuntimeSettings::load(&setup.runtime_settings.stored_path).unwrap(); diff --git a/updatehub/src/states/validation.rs b/updatehub/src/states/validation.rs index 37a2235f..924dd61a 100644 --- a/updatehub/src/states/validation.rs +++ b/updatehub/src/states/validation.rs @@ -33,16 +33,13 @@ impl StateChangeImpl for Validation { async fn handle(self, context: &mut Context) -> Result<(State, machine::StepTransition)> { if let Some(key) = context.firmware.pub_key.as_ref() { - match self.sign.as_ref() { - Some(sign) => { - debug!("validating signature"); - sign.validate(key, &self.package) - .log_error_msg("uhupkg failed signature validation")?; - } - None => { - error!("missing signature key"); - return Err(super::TransitionError::SignatureNotFound); - } + if let Some(sign) = self.sign.as_ref() { + debug!("validating signature"); + sign.validate(key, &self.package) + .log_error_msg("uhupkg failed signature validation")?; + } else { + error!("missing signature key"); + return Err(super::TransitionError::SignatureNotFound); } } else { info!("no signature key available on device, ignoring signature validation"); @@ -70,7 +67,7 @@ impl StateChangeImpl for Validation { self.package .validate_install_modes(&context.settings, inactive_installation_set) .log_error_msg("install mode failed validation")?; - for obj in self.package.objects(inactive_installation_set).iter() { + for obj in self.package.objects(inactive_installation_set) { if let Err(e) = obj.check_requirements(&object_context).await { error!( "update package: {} ({}) has failed to meet the install requirements", @@ -87,8 +84,7 @@ impl StateChangeImpl for Validation { if context .runtime_settings .applied_package_uid() - .map(|u| *u == update_package.package_uid()) - .unwrap_or_default() + .is_some_and(|u| *u == update_package.package_uid()) { info!("not downloading update package, the same package has already been installed"); Ok((State::EntryPoint(EntryPoint {}), machine::StepTransition::Immediate)) @@ -115,10 +111,10 @@ impl StateChangeImpl for Validation { for object in not_ready { match object { (filename, Ok(status)) => { - error!(" file '{}' is {:?}", filename, status) + error!(" file '{}' is {:?}", filename, status); } (filename, Err(err)) => { - error!(" file '{}' has failed with error: {:?}", filename, err) + error!(" file '{}' has failed with error: {:?}", filename, err); } } } diff --git a/updatehub/src/tests.rs b/updatehub/src/tests.rs index a80319e1..6fcd286e 100644 --- a/updatehub/src/tests.rs +++ b/updatehub/src/tests.rs @@ -126,10 +126,7 @@ impl TestEnvironmentBuilder { hardware_hook(dir_path), &format!( "#!/bin/sh\necho {}", - match self.invalid_hardware { - false => "board", - true => "invalid", - } + if self.invalid_hardware { "invalid" } else { "board" } ), ); create_hook( @@ -166,7 +163,7 @@ impl TestEnvironmentBuilder { create_hook(validate_hook(&firmware.stored_path), &script); } - for bin in self.extra_binaries.into_iter() { + for bin in self.extra_binaries { let mut file = fs::File::create(bin_dir_path.join(&bin)).unwrap(); writeln!(file, "#!/bin/sh\necho {} $@ >> {}", bin, output_file.to_string_lossy()) .unwrap(); @@ -192,7 +189,7 @@ impl TestEnvironmentBuilder { fs::remove_file(&file_path).unwrap(); let mut runtime_settings = RuntimeSettings::default(); - runtime_settings.path = file_path.clone(); + runtime_settings.path.clone_from(&file_path); if self.booting_from_update { runtime_settings.enable_persistency(); runtime_settings.set_upgrading_to(Set(InstallationSet::A)).unwrap(); diff --git a/updatehub/src/update_package/mod.rs b/updatehub/src/update_package/mod.rs index fa2fdb0a..d568ff26 100644 --- a/updatehub/src/update_package/mod.rs +++ b/updatehub/src/update_package/mod.rs @@ -70,7 +70,7 @@ impl UpdatePackageExt for UpdatePackage { if let Some(mode) = self .objects(installation_set) .iter() - .map(|o| o.mode()) + .map(super::object::info::Info::mode) .find(|mode| !install_modes.contains(mode)) { return Err(Error::IncompatibleInstallMode(mode)); @@ -104,7 +104,7 @@ impl UpdatePackageExt for UpdatePackage { .filter(|o| { o.status(&settings.update.download_dir) .map_err(|e| { - error!("fail accessing the object: {} (err: {})", o.sha256sum(), e) + error!("fail accessing the object: {} (err: {})", o.sha256sum(), e); }) .unwrap_or(object::info::Status::Missing) .eq(&filter) diff --git a/updatehub/src/utils/definitions.rs b/updatehub/src/utils/definitions.rs index 13597bdc..fd7663ab 100644 --- a/updatehub/src/utils/definitions.rs +++ b/updatehub/src/utils/definitions.rs @@ -28,11 +28,11 @@ impl Access { } } -/// Utility functions for [TargetType](pkg_schema::definitions::TargetType) +/// Utility functions for [`TargetType`](pkg_schema::definitions::TargetType) pub(crate) trait TargetTypeExt { /// Checks whether the device is valid to start installation, i.e., /// device exists, use have write permission, and is free of mounted - /// filesystems when the handler asks for [Access::Exclusive]. + /// filesystems when the handler asks for [`Access::Exclusive`]. fn valid(&self, access: Access) -> Result<&Self>; /// Gets device's path for mounting. diff --git a/updatehub/src/utils/fs.rs b/updatehub/src/utils/fs.rs index 529bacf3..a76daf83 100644 --- a/updatehub/src/utils/fs.rs +++ b/updatehub/src/utils/fs.rs @@ -140,7 +140,7 @@ fn parse_mount_entry(line: &str) -> Option { Some(MountEntry { device: (major.parse().ok()?, minor.parse().ok()?), - source: fields.get(separator + 2)?.to_string(), + source: (*fields.get(separator + 2)?).to_string(), mount_point: PathBuf::from(fields.get(4)?), }) } @@ -206,9 +206,10 @@ pub(crate) fn is_executable_in_path(cmd: &str) -> Result<()> { #[cfg(test)] if let Some(dirs) = search_path::current() { - return match dirs.iter().any(|dir| is_executable_file(&dir.join(cmd))) { - true => Ok(()), - false => Err(Error::ExecutableNotInPath(cmd.to_owned())), + return if dirs.iter().any(|dir| is_executable_file(&dir.join(cmd))) { + Ok(()) + } else { + Err(Error::ExecutableNotInPath(cmd.to_owned())) }; } @@ -273,18 +274,18 @@ pub(crate) mod search_path { } pub(super) fn current() -> Option> { - OVERRIDE.with_borrow(|o| o.clone()) + OVERRIDE.with_borrow(std::clone::Clone::clone) } } -pub(crate) fn format(target: &Path, fs: Filesystem, options: &Option) -> Result<()> { +pub(crate) fn format(target: &Path, fs: Filesystem, options: Option<&str>) -> Result<()> { // The commands below are forced so they run unattended, which also means // they will not refuse to wipe a mounted filesystem on their own. ensure_not_mounted(target)?; trace!("formating {:?} as {}", target, fs); let target = target.display(); - let options = options.clone().unwrap_or_default(); + let options = options.unwrap_or_default(); let cmd = match fs { Filesystem::Jffs2 => format!("flash_erase -j {options} {target} 0 0"), @@ -330,12 +331,12 @@ pub(crate) fn chmod(path: &Path, mode: u32) -> Result<()> { Ok(()) } -pub(crate) fn chown(path: &Path, uid: &Option, gid: &Option) -> Result<()> { +pub(crate) fn chown(path: &Path, uid: Option<&Uid>, gid: Option<&Gid>) -> Result<()> { trace!("applying ownership of uid:{:?} and gid:{:?} to {:?}", uid, gid, path); Ok(nix::unistd::chown( path, - uid.as_ref().map(|id| nix::unistd::Uid::from_raw(id.as_u32())), - gid.as_ref().map(|id| nix::unistd::Gid::from_raw(id.as_u32())), + uid.map(|id| nix::unistd::Uid::from_raw(id.as_u32())), + gid.map(|id| nix::unistd::Gid::from_raw(id.as_u32())), )?) } @@ -468,9 +469,8 @@ mod tests { )); } - // Requires root, as creating a loop device does. #[test] - #[ignore] + #[ignore = "needs root to attach a loop device"] fn device_nodes_are_measured_by_their_own_capacity() { let loop_device = FakeLoopDevice::new(LOOP_DEVICE_SIZE).unwrap(); @@ -526,9 +526,8 @@ mod tests { assert!(ensure_not_mounted(Path::new("/dev/updatehub-inexistent-device")).is_ok()); } - // Requires root, as creating a loop device does. #[test] - #[ignore] + #[ignore = "needs root to attach a loop device"] fn mounted_device_is_in_use() { let loop_device = FakeLoopDevice::new(LOOP_DEVICE_SIZE).unwrap(); @@ -537,7 +536,7 @@ mod tests { "an unmounted loop device should be free to install onto" ); - format(&loop_device.device, Filesystem::Ext4, &None).unwrap(); + format(&loop_device.device, Filesystem::Ext4, None).unwrap(); let guard = mount(&loop_device.device, Filesystem::Ext4, "").unwrap(); let in_use = ensure_not_mounted(&loop_device.device); diff --git a/updatehub/src/utils/mtd.rs b/updatehub/src/utils/mtd.rs index 54533c80..9270c331 100644 --- a/updatehub/src/utils/mtd.rs +++ b/updatehub/src/utils/mtd.rs @@ -16,10 +16,7 @@ pub(crate) fn target_device_from_ubi_volume_name(volume: &str) -> Result> = Arc::new(Mutex::default()); - } + pub static SERIALIZE: LazyLock>> = LazyLock::new(|| Arc::new(Mutex::default())); #[test] - #[ignore] + #[ignore = "needs root to load the MTD simulator modules"] fn device_from_mtd_name() { let _lock = SERIALIZE.lock(); let dev_names = vec!["system0", "system1"]; @@ -289,7 +283,7 @@ pub(crate) mod tests { } #[test] - #[ignore] + #[ignore = "needs root to load the MTD simulator modules"] fn test_is_nand() { let _lock = SERIALIZE.lock(); @@ -304,7 +298,7 @@ pub(crate) mod tests { } #[test] - #[ignore] + #[ignore = "needs root to load the MTD simulator modules"] fn device_from_ubi_volume_name() { let _lock = SERIALIZE.lock(); let volume_names = vec!["some_ui_volume", "another_ubi_volume"]; @@ -321,7 +315,7 @@ pub(crate) mod tests { } #[test] - #[ignore] + #[ignore = "needs root to load the MTD simulator modules"] fn device_from_ubi_volume_name_multiple_volumes() { let _lock = SERIALIZE.lock(); let volume_names = vec![ diff --git a/updatehub/src/utils/test_env.rs b/updatehub/src/utils/test_env.rs index fc3d80c6..41d251eb 100644 --- a/updatehub/src/utils/test_env.rs +++ b/updatehub/src/utils/test_env.rs @@ -6,17 +6,14 @@ //! only needs `is_executable_in_path` to see other directories. It changes no //! process state at all. -use lazy_static::lazy_static; use std::{ env, ffi::OsString, path::Path, - sync::{Mutex, MutexGuard}, + sync::{LazyLock, Mutex, MutexGuard}, }; -lazy_static! { - static ref ENV_LOCK: Mutex<()> = Mutex::default(); -} +static ENV_LOCK: LazyLock> = LazyLock::new(Mutex::default); /// Holds `PATH` at a test-defined value, and restores it on drop. /// @@ -59,7 +56,7 @@ impl PathEnvGuard { fn acquire() -> (MutexGuard<'static, ()>, Option) { // A panicking test poisons the lock, but `Drop` still restored `PATH` // during the unwind, so the poison carries no information. - let lock = ENV_LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + let lock = ENV_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); let previous = env::var_os("PATH"); (lock, previous) diff --git a/updatehub/tests/common.rs b/updatehub/tests/common.rs index 3fb835bf..2cb5f31f 100644 --- a/updatehub/tests/common.rs +++ b/updatehub/tests/common.rs @@ -79,10 +79,10 @@ impl Settings { setup = setup.validate_callback(s.to_owned()); } if let Some(l) = self.install_modes { - setup = setup.supported_install_modes(l) + setup = setup.supported_install_modes(l); } if !self.polling { - setup = setup.disable_polling() + setup = setup.disable_polling(); } let mut setup = setup.finish(); @@ -380,27 +380,23 @@ pub fn rewrite_log_output(s: String) -> (String, String) { let version_re = Regex::new(r"Agent .*").unwrap(); let tmpfile_re = Regex::new(r#""/.*/.tmp.*""#).unwrap(); let date_re = Regex::new(r"\b(?:Jan|...|Dec) (\d{2}) (\d{2}):(\d{2}):(\d{2}).(\d{3})").unwrap(); - let time_re = Regex::new(r#"(\d{5}) seconds"#).unwrap(); + let time_re = Regex::new(r"(\d{5}) seconds").unwrap(); let trce_re = Regex::new(r" TRCE.*").unwrap(); let debg_re = Regex::new(r" DEBG.*").unwrap(); - let download_re = Regex::new(r"DEBG (\d{2})%").unwrap(); + let download_re = Regex::new(r"DEBG (\d{1,3})%").unwrap(); let s = server_address_re.replace_all(&s, "http://127.0.0.1:[port]"); let s = version_re.replace_all(&s, "Agent "); let s = tmpfile_re.replace_all(&s, r#""""#); let s = date_re.replace_all(&s, ""); let s = download_re.replace_all(&s, "DEBG %"); - let s = time_re.replace_all(&s, r#"