Use cfg_if macro

There were a few places where this can be used for a slight improvement to code quality.
paritytech · Sep 25, 2023 · 279db25 · 279db25
1 parent ad0ed8a
commit 279db25
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 81 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/polkadot/node/core/pvf/Cargo.toml b/polkadot/node/core/pvf/Cargo.toml
@@ -8,6 +8,7 @@ license.workspace = true
 
 [dependencies]
 always-assert = "0.1"
+cfg-if = "1.0"
 futures = "0.3.21"
 futures-timer = "3.0.2"
 gum = { package = "tracing-gum", path = "../../gum" }

diff --git a/polkadot/node/core/pvf/common/Cargo.toml b/polkadot/node/core/pvf/common/Cargo.toml
@@ -7,6 +7,7 @@ edition.workspace = true
 license.workspace = true
 
 [dependencies]
+cfg-if = "1.0"
 cpu-time = "1.0.0"
 futures = "0.3.21"
 gum = { package = "tracing-gum", path = "../../../gum" }

diff --git a/polkadot/node/core/pvf/prepare-worker/Cargo.toml b/polkadot/node/core/pvf/prepare-worker/Cargo.toml
@@ -7,6 +7,7 @@ edition.workspace = true
 license.workspace = true
 
 [dependencies]
+cfg-if = "1.0"
 futures = "0.3.21"
 gum = { package = "tracing-gum", path = "../../../gum" }
 libc = "0.2.139"

diff --git a/polkadot/node/core/pvf/prepare-worker/src/lib.rs b/polkadot/node/core/pvf/prepare-worker/src/lib.rs
@@ -212,10 +212,13 @@ pub fn worker_entrypoint(
 								Err(err)
 							},
 							Ok(ok) => {
-								#[cfg(not(target_os = "linux"))]
-								let (artifact, cpu_time_elapsed) = ok;
-								#[cfg(target_os = "linux")]
-								let (artifact, cpu_time_elapsed, max_rss) = ok;
+								cfg_if::cfg_if! {
+									if #[cfg(target_os = "linux")] {
+										let (artifact, cpu_time_elapsed, max_rss) = ok;
+									} else {
+										let (artifact, cpu_time_elapsed) = ok;
+									}
+								}
 
 								// Stop the memory stats worker and get its observed memory stats.
 								#[cfg(any(target_os = "linux", feature = "jemalloc-allocator"))]

diff --git a/polkadot/node/core/pvf/src/host.rs b/polkadot/node/core/pvf/src/host.rs
@@ -891,48 +891,46 @@ fn check_can_unshare_user_namespace_and_change_root(
 	#[cfg_attr(not(target_os = "linux"), allow(unused_variables))]
 	prepare_worker_program_path: &Path,
 ) -> bool {
-	#[cfg(target_os = "linux")]
-	{
-		let output = std::process::Command::new(prepare_worker_program_path)
-			.arg("--check-can-unshare-user-namespace-and-change-root")
-			.output();
-
-		match output {
-			Ok(output) if output.status.success() => true,
-			Ok(output) => {
-				let stderr = std::str::from_utf8(&output.stderr)
-					.expect("child process writes a UTF-8 string to stderr; qed")
-					.trim();
-				gum::warn!(
-					target: LOG_TARGET,
-					?prepare_worker_program_path,
-					// Docs say to always print status using `Display` implementation.
-					status = %output.status,
-					%stderr,
-					"Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running with support for unsharing user namespaces for maximum security."
-				);
-				false
-			},
-			Err(err) => {
-				gum::warn!(
-					target: LOG_TARGET,
-					?prepare_worker_program_path,
-					"Could not start child process: {}",
-					err
-				);
-				false
-			},
+	cfg_if::cfg_if! {
+		if #[cfg(target_os = "linux")] {
+			let output = std::process::Command::new(prepare_worker_program_path)
+				.arg("--check-can-unshare-user-namespace-and-change-root")
+				.output();
+
+			match output {
+				Ok(output) if output.status.success() => true,
+				Ok(output) => {
+					let stderr = std::str::from_utf8(&output.stderr)
+						.expect("child process writes a UTF-8 string to stderr; qed")
+						.trim();
+					gum::warn!(
+						target: LOG_TARGET,
+						?prepare_worker_program_path,
+						// Docs say to always print status using `Display` implementation.
+						status = %output.status,
+						%stderr,
+						"Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running with support for unsharing user namespaces for maximum security."
+					);
+					false
+				},
+				Err(err) => {
+					gum::warn!(
+						target: LOG_TARGET,
+						?prepare_worker_program_path,
+						"Could not start child process: {}",
+						err
+					);
+					false
+				},
+			}
+		} else {
+			gum::warn!(
+				target: LOG_TARGET,
+				"Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with support for unsharing user namespaces for maximum security."
+			);
+			false
 		}
 	}
-
-	#[cfg(not(target_os = "linux"))]
-	{
-		gum::warn!(
-			target: LOG_TARGET,
-			"Cannot unshare user namespace and change root, which are Linux-specific kernel security features. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with support for unsharing user namespaces for maximum security."
-		);
-		false
-	}
 }
 
 /// Check if landlock is supported and emit a warning if not.
@@ -944,45 +942,43 @@ fn check_landlock(
 	#[cfg_attr(not(target_os = "linux"), allow(unused_variables))]
 	prepare_worker_program_path: &Path,
 ) -> bool {
-	#[cfg(target_os = "linux")]
-	{
-		match std::process::Command::new(prepare_worker_program_path)
-			.arg("--check-can-enable-landlock")
-			.status()
-		{
-			Ok(status) if status.success() => true,
-			Ok(status) => {
-				let abi =
-					polkadot_node_core_pvf_common::worker::security::landlock::LANDLOCK_ABI as u8;
-				gum::warn!(
-					target: LOG_TARGET,
-					?prepare_worker_program_path,
-					?status,
-					%abi,
-					"Cannot fully enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider upgrading the kernel version for maximum security."
-				);
-				false
-			},
-			Err(err) => {
-				gum::warn!(
-					target: LOG_TARGET,
-					?prepare_worker_program_path,
-					"Could not start child process: {}",
-					err
-				);
-				false
-			},
+	cfg_if::cfg_if! {
+		if #[cfg(target_os = "linux")] {
+			match std::process::Command::new(prepare_worker_program_path)
+				.arg("--check-can-enable-landlock")
+				.status()
+			{
+				Ok(status) if status.success() => true,
+				Ok(status) => {
+					let abi =
+						polkadot_node_core_pvf_common::worker::security::landlock::LANDLOCK_ABI as u8;
+					gum::warn!(
+						target: LOG_TARGET,
+						?prepare_worker_program_path,
+						?status,
+						%abi,
+						"Cannot fully enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider upgrading the kernel version for maximum security."
+					);
+					false
+				},
+				Err(err) => {
+					gum::warn!(
+						target: LOG_TARGET,
+						?prepare_worker_program_path,
+						"Could not start child process: {}",
+						err
+					);
+					false
+				},
+			}
+		} else {
+			gum::warn!(
+				target: LOG_TARGET,
+				"Cannot enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with landlock support for maximum security."
+			);
+			false
 		}
 	}
-
-	#[cfg(not(target_os = "linux"))]
-	{
-		gum::warn!(
-			target: LOG_TARGET,
-			"Cannot enable landlock, a Linux-specific kernel security feature. Running validation of malicious PVF code has a higher risk of compromising this machine. Consider running on Linux with landlock support for maximum security."
-		);
-		false
-	}
 }
 
 #[cfg(test)]