Skip to content

Commit 15a7d59

Browse files
refactor(file): replace fdisk with gptman+mbrman (#154)
## Summary Replace the fragile `fdisk`-based partition detection in `get_partition_info()` with Rust-native `gptman` and `mbrman` crates. **Changes:** - Add `gptman` (GPT with CRC32 validation) and `mbrman` (MBR including logical partitions) as dependencies - Add `src/file/partition.rs` with `get_partition_data()` and `is_gpt()` - Remove locale-sensitive `fdisk` shell invocation and regex parsing - Remove `fdisk` from `package.metadata.deb` depends - Change `PartitionInfo` fields from `String` to typed `u32`/`u64` ## Reason The previous implementation spawned `fdisk -l` and parsed its text output with regex — making it locale-sensitive, version-dependent, and requiring `fdisk` as a runtime system dependency. Additionally, a pre-existing bug was fixed: `PartitionInfo.end` held the last sector (inclusive) from `fdisk` output and was passed directly as `dd count=`, but `count` expects the number of sectors. For a partition starting at sector 2048 and ending at 206847, this caused `dd` to copy 206847 sectors instead of the correct 204800. **Fixed semantics:** - GPT: `count = ending_lba - starting_lba + 1` - MBR: `count = p.sectors` (already the sector count) ## Verification - `cargo check` passes - `cargo fmt -- --check` passes - `cargo clippy -- -D warnings` passes - Manually tested against GPT and MBR image files --------- Signed-off-by: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent a672d94 commit 15a7d59

5 files changed

Lines changed: 195 additions & 70 deletions

File tree

Cargo.lock

Lines changed: 101 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ license = "MIT OR Apache-2.0"
77
name = "omnect-cli"
88
readme = "README.md"
99
repository = "https://github.com/omnect/omnect-cli"
10-
version = "0.27.1"
10+
version = "0.27.2"
1111

1212
[dependencies]
1313
actix-web = "4.11"
@@ -37,6 +37,8 @@ filemagic = { version = "0.13", default-features = false, features = [
3737
"pkg-config",
3838
] }
3939
flate2 = { version = "1.1", default-features = false }
40+
gptman = { version = "1.1", default-features = false }
41+
mbrman = { version = "0.5", default-features = false }
4042
omnect-crypto = { git = "https://github.com/omnect/omnect-crypto.git", tag = "0.4.0" }
4143
keyring = { version = "3.6", default-features = false }
4244
libfs = { version = "0.9", default-features = false }
@@ -85,5 +87,5 @@ tar = "0.4"
8587

8688
# metadata for building with cargo-deb (https://crates.io/crates/cargo-deb)
8789
[package.metadata.deb]
88-
depends = "bmap-tools, e2tools, fdisk, keychain, libc6 (>= 2.34), libmagic1, libssl3 (>= 3.0.0), mtools"
90+
depends = "bmap-tools, e2tools, keychain, libc6 (>= 2.34), libmagic1, libssl3 (>= 3.0.0), mtools"
8991
revision = ""

src/file/functions.rs

Lines changed: 28 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use anyhow::{Context, Result};
22
use log::{debug, warn};
3-
use regex::Regex;
43
use std::collections::HashMap;
54
use std::fmt::{self, Display};
65
use std::fs;
@@ -22,9 +21,9 @@ pub enum Partition {
2221

2322
#[derive(Debug)]
2423
struct PartitionInfo {
25-
num: String,
26-
start: String,
27-
end: String,
24+
num: u32,
25+
start: u64,
26+
count: u64,
2827
}
2928

3029
impl Display for Partition {
@@ -184,23 +183,6 @@ macro_rules! try_exec_cmd {
184183
};
185184
}
186185

187-
macro_rules! exec_cmd_with_output {
188-
($cmd:expr) => {{
189-
let res = $cmd
190-
.output()
191-
.context(format!("{}: spawn {:?}", function_name!(), $cmd))?;
192-
193-
let output =
194-
String::from_utf8(res.stdout).context(format!("{}: get output", function_name!()))?;
195-
196-
let output = output.trim();
197-
198-
debug!("{}: {:?}", function_name!(), $cmd);
199-
200-
output.to_string()
201-
}};
202-
}
203-
204186
pub fn copy_to_image(file_copy_params: &[FileCopyToParams], image_file: &Path) -> Result<()> {
205187
// we use the folder the image is located in
206188
// the caller is responsible to create a /tmp/ directory if needed
@@ -381,59 +363,39 @@ pub fn read_file_from_image(
381363
}
382364

383365
fn get_partition_info(image_file: &str, partition: &Partition) -> Result<PartitionInfo> {
384-
let mut fdisk = Command::new("fdisk");
385-
fdisk
386-
.arg("-l")
387-
.arg("-o")
388-
.arg("Device,Start,End")
389-
.arg(image_file);
390-
let fdisk_out = exec_cmd_with_output!(fdisk);
366+
use crate::file::partition::{get_partition_data, is_gpt};
391367

392-
let partition_num = match partition {
368+
let gpt =
369+
is_gpt(image_file).context("get_partition_info: failed to detect partition table type")?;
370+
371+
let partition_num: u32 = match partition {
393372
Partition::boot => 1,
394373
Partition::rootA => 2,
395-
p @ (Partition::factory | Partition::cert) => {
396-
let re = Regex::new(r"Disklabel type: (\D{3})").unwrap();
397-
398-
let matches = re
399-
.captures(&fdisk_out)
400-
.context("get_partition_info: regex no matches found")?;
401-
anyhow::ensure!(
402-
matches.len() == 2,
403-
"'get_partition_info: regex contains unexpected number of matches"
404-
);
405-
406-
let partition_type = &matches[1];
407-
408-
debug!("partition type: {partition_type}");
409-
410-
match (p, partition_type) {
411-
(Partition::factory, "gpt") => 4,
412-
(Partition::factory, "dos") => 5,
413-
(Partition::cert, "gpt") => 5,
414-
(Partition::cert, "dos") => 6,
415-
_ => anyhow::bail!("get_partition_info: unhandled partition type"),
374+
Partition::factory => {
375+
if gpt {
376+
4
377+
} else {
378+
5
379+
}
380+
}
381+
Partition::cert => {
382+
if gpt {
383+
5
384+
} else {
385+
6
416386
}
417387
}
418388
};
419389

420-
let re = Regex::new(format!(r"{image_file}{partition_num}\s+(\d+)\s+(\d+)").as_str())
421-
.context("get_partition_info: failed to create regex")?;
422-
423-
let matches = re
424-
.captures(&fdisk_out)
425-
.context("get_partition_info: regex no matches found")?;
426-
anyhow::ensure!(
427-
matches.len() == 3,
428-
"'get_partition_info: regex contains unexpected number of matches"
429-
);
390+
debug!("get_partition_info: partition={partition}, num={partition_num}, gpt={gpt}");
430391

431-
let partition_offset = (matches[1].to_string(), matches[2].to_string());
392+
let data = get_partition_data(image_file, partition_num)
393+
.with_context(|| format!("get_partition_info: failed to read partition {partition_num}"))?;
432394

433395
let info = PartitionInfo {
434-
num: partition_num.to_string(),
435-
start: partition_offset.0,
436-
end: partition_offset.1,
396+
num: data.num,
397+
start: data.start,
398+
count: data.count,
437399
};
438400

439401
debug!("get_partition_info: {:?}", info);
@@ -455,7 +417,7 @@ fn read_partition(
455417
.arg(format!("of={partition_file}"))
456418
.arg("bs=512")
457419
.arg(format!("skip={}", partition_info.start))
458-
.arg(format!("count={}", partition_info.end))
420+
.arg(format!("count={}", partition_info.count))
459421
.arg("conv=sparse")
460422
.arg("status=none");
461423
exec_cmd!(dd);
@@ -476,7 +438,7 @@ fn write_partition(
476438
.arg(format!("of={image_file}"))
477439
.arg("bs=512")
478440
.arg(format!("seek={}", partition_info.start))
479-
.arg(format!("count={}", partition_info.end))
441+
.arg(format!("count={}", partition_info.count))
480442
.arg("conv=notrunc,sparse")
481443
.arg("status=none");
482444
exec_cmd!(dd);

src/file/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
pub mod compression;
22
pub mod functions;
3+
mod partition;
34
use super::validators::{
45
device_update,
56
identity::{IdentityConfig, IdentityType, validate_identity},

0 commit comments

Comments
 (0)