Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/mach/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub mod load_command;
pub mod relocation;
pub mod segment;
pub mod symbols;
pub mod version;

pub use self::constants::cputype;

Expand Down
148 changes: 148 additions & 0 deletions src/mach/version.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
/*-
* Copyright: see LICENSE file
*/

use crate::mach::MachO;
use crate::mach::load_command::CommandVariant;

if_std! {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you can remove this entire block once you get rid of the allocations

use crate::error;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc, error is not under std; and I don't think cmp or str or fmt is either, they should be under core?

use crate::mach::{Mach, SingleArch};
use crate::mach::cputype::CpuType;
use std::cmp::Ordering;
use std::collections::{VecDeque,HashMap};
use std::str::FromStr;
use std::fmt;
}

#[derive(Eq, Debug)]
pub struct Version {
pub major: u32,
pub minor: u32,
pub patch: u32,
Comment on lines +19 to +22

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please document all of this, public things, especially new ones, should be doc'd

}

impl From<u32> for Version {
fn from(packed: u32) -> Self {
// X.Y.Z is encoded in nibbles xxxx.yy.zz
// 12.6 = 0b0000_0000_0000_1100_0000_0110_0000_0000
Self {
major: (packed & 0b1111_1111_1111_1111_0000_0000_0000_0000u32) >> 16,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i think we use hex for other bitmasks in other parts of codebase, might want to be uniform with that

minor: (packed & 0b0000_0000_0000_0000_1111_1111_0000_0000u32) >> 8,
patch: (packed & 0b0000_0000_0000_0000_0000_0000_1111_1111u32) >> 0,
}
}
}

impl MachO<'_> {
pub fn version(&self) -> Option<Version> {
self.load_commands
.iter()
.find_map(|c| match c.command {
CommandVariant::VersionMinMacosx(v) => Some(v.version),
CommandVariant::BuildVersion(v) => Some(v.minos),
_ => None,
})
.map(Version::from)
}
}

impl PartialEq for Version {
fn eq(&self, other: &Self) -> bool {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should be able to just #[derive(PartialEq)

self.major == other.major && self.minor == other.minor && self.patch == other.patch
}
}

if_std! {
impl fmt::Display for Version {
// This trait requires `fmt` with this exact signature.
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
// Write strictly the first element into the supplied output
// stream: `f`. Returns `fmt::Result` which indicates whether the
// operation succeeded or failed. Note that `write!` uses syntax which
// is very similar to `println!`.
write!(f, "{}.{}.{}", self.major, self.minor, self.patch)
}
}

impl Ord for Version {
fn cmp(&self, other: &Self) -> Ordering {
let mao = self.major.cmp(&other.major);
let mio = self.minor.cmp(&other.minor);
let pao = self.patch.cmp(&other.patch);
if mao == Ordering::Equal && mio == Ordering::Equal {
pao
} else if mao == Ordering::Equal {
mio
} else {
mao
}
}
}

impl PartialOrd for Version {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}
Comment on lines +68 to +87

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, you should be able to derive this (and it also doesn't require std)


impl FromStr for Version {
type Err = error::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
Comment thread
CamJN marked this conversation as resolved.
let mut parts = s
.trim()
.split('.')
.map(|p| p.parse::<u32>().unwrap_or(0))
.take(3)
.collect::<VecDeque<u32>>();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this allocates a VecDequeu here just to pop the front; i think you can rewrite this to not allocate, and please do that. thank you for updating with fallible methods though.


if parts.front().is_some_and(|major| *major > 0) {
Ok(Self {
major: parts.pop_front().unwrap(), // existance checked in conditional
minor: parts.pop_front().unwrap_or(0),
patch: parts.pop_front().unwrap_or(0),
})
} else {
Err(error::Error::Malformed("Missing major version from target version, version string should look like: X.Y.Z".to_string()))
}
}
}

impl TryFrom<Mach<'_>> for Vec<Version> {
type Error = error::Error;

fn try_from(b: Mach) -> Result<Self, error::Error> {
match b {
Mach::Binary(b) => b.version().ok_or(error::Error::Malformed("Binary has no version".to_string())).map(|v|vec![v]),
Mach::Fat(f) => f.into_iter().map(|r| r.map(|s| match s {
SingleArch::MachO(b) => b.version().ok_or_else(||error::Error::Malformed("Missing or corrupted version".to_string())),
SingleArch::Archive(_) => Err(error::Error::Malformed("lib is an archive?".to_string())),
}).and_then(std::convert::identity)).collect(),
}
}
}

impl Mach<'_> {
pub fn versions(self) -> HashMap<CpuType, Version> {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs docstring

In general, I don't necessarily like a method hanging off of Mach here in this file, as it's not local anymore to Mach. This also allocates a hashmap; I think it would be better to probably return an impl Iterator<(CpuType, Version)>; this way users can allocate if they want, etc.

let mut hash = HashMap::new();
match self {
Mach::Binary(b) => {
if let Some(v) = b.version() {
hash.insert(b.header.cputype, v);
}
},
Mach::Fat(f) => {
for r in f.into_iter() {
if let Ok(SingleArch::MachO(b)) = r {
if let Some(v) = b.version() {
hash.insert(b.header.cputype, v);
}
}
}
},
};
hash
}
}
}
25 changes: 25 additions & 0 deletions tests/macho.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,31 @@ fn parse_sections() {
}
}

#[test]
fn version() {
let bytes = &DEADBEEF_MACH_64;
let mach = Mach::parse(&bytes[..]).unwrap();
let actual_hash = mach.versions();
let actual = &actual_hash[&cputype::CPU_TYPE_X86_64];
let expected = "10.10.0".parse::<version::Version>();
assert!(expected.is_ok()); // Test parsing strings
assert_eq!(actual_hash.len(), 1); // Test parsing binaries
assert_eq!(actual, &expected.unwrap()); // Test parsing binaries
assert_eq!(format!("{}", actual), "10.10.0"); // Test formatting version
}

#[test]
fn version_from_macho() {
let bytes = &DEADBEEF_MACH_64;
if let Mach::Binary(binary) = Mach::parse(bytes.as_ref()).unwrap() {
let expected = "10.10.0".parse::<version::Version>().unwrap();
let actual = binary.version();
assert_eq!(actual, Some(expected));
} else {
panic!("got mach fat from regular binary");
}
}

#[test]
fn iter_symbols() {
let bytes = &DEADBEEF_MACH_64;
Expand Down
Loading