upgrade magic-sys to 0.4.2 and add get/set param functions

[rainmote]

References

Description

1. upgrade magic-sys to 0.4.2
2. add get/set param functions

[codecov]

Codecov Report

❌ Patch coverage is 53.84615% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.01%. Comparing base (ee436b0) to head (d4a214e).
⚠️ Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
src/lib.rs	58.13%	36 Missing ⚠️
src/ffi.rs	41.93%	18 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #433      +/-   ##
==========================================
+ Coverage   47.86%   49.01%   +1.15%     
==========================================
  Files           2        2              
  Lines         491      608     +117     
  Branches      491      608     +117     
==========================================
+ Hits          235      298      +63     
- Misses        255      309      +54     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[robo9k]

Thanks for the merge request, params are a milestone to get magic feature complete!

However there's some changes I'd like to see:

The libmagic man page describes a size_t type for each param, yet the functions use void *
To me this suggests that future params might have a different type, but the suggested Rust code uses c_int for all params
I haven't thought about a design in-depth, but it feels more rust-y to have e.g. a enum ParamKind (the current Param) for getting and a enum Param for setting, which has a variant with - currently - a c_size_t for each kind.

Roughly this:

enum ParamKind {
    IndirMax,
}

enum Param {
    IndirMax(c_size_t),
}

fn get_param(&self, param: ParamKind) -> Result<Param, GetParamError> {}

fn set_param(&self, param: &Param) -> Result<(), SetParamError> {}

The suggested code uses c_int instead of (unstable) c_size_t, which are not strictly equivalent on some platforms

There's some smaller things like documentation about params not being updated elsewhere, bumping the toolchain version without MSRV, cargo fmt and other CI checks

I like the consistency with other code and docs and tests!

There might be other changes that I didn't think of yet

How do you want to proceed?
We can do a bit of back and forth with me reviewing and suggesting and eventually taking your entire evolved code,
but if you're not in a hurry and just want params as a feature, I might get around to implementing them myself soon as well - partially using your current code as a basis

[robo9k]

On second thought the enum is unergonomic because users have to match on it
Maybe (sealed) trait Param and separate types;

unsafe trait Param {
    fn kind() -> c_int;
    fn uninit() -> Self;
    unsafe fn as_mut_c_void(&mut self) -> *mut c_void;
    unsafe fn as_c_void(&self) -> *const c_void;
}

pub struct IndirMax(pub c_size_t)
impl Param for IndirMax {
    fn kind() -> c_int {
        MAGIC_PARAM_INDIR_MAX
    }
    
    fn uninit() -> Self {
        Self(0)
    }
    
    fn as_mut_c_void(&mut self) -> *mut c_void {
        self.0 as *mut c_size_t as *mut _
    }
    
    fn as_c_void(&self) -> *const c_void {
        self.0 as *const c_size_t as *const _
    }
}

fn get_param<P: Param>(&self) -> Result<P, GetParamError> {
        let mut param = P::uninit();
        
        unsafe { magic_getparam(P::kind(), param.as_mut_c_void()); }
        
        Ok(param)
}
fn set_param<P: Param>(&self, param: &P) -> Result<(), SetParamError> {
        unsafe { magic_setparam(P::kind(), param.as_c_void()); }
        Ok(())
}

let indir_max = cookie.get_param::<IndirMax>()?;
cookie.set_param(&IndirMax(42))?;

kind and uninit are not unsafe
Other methods could be named as_ptr and as_mut_ptr

More elaborate would be Param::Storage (being c_size_t currently) associated type, MaybeUninit<Storage> and TryFrom<Storage>

[robo9k]

Behold my latest design idea

type ParamKind = c_int;

/// # Safety
///
/// `Self::Value` must match the type `libmagic` is expecting for `Self::KIND`
unsafe trait Param {
    type Value;
    const KIND: ParamKind;

    fn from_value(value: Self::Value) -> Self;

    fn as_ptr(&self) -> *const Self::Value;
}

pub struct IndirMax(pub c_size_t);

unsafe impl Param for IndirMax {
    type Value = c_size_t;
    const KIND: ParamKind = MAGIC_PARAM_INDIR_MAX;

    fn from_value(value: Self::Value) -> Self {
        Self(value)
    }

    fn as_ptr(&self) -> *const Self::Value {
        &self.0 as *const _
    }
}

impl Cookie {
    pub fn get_param<P: Param>(&self) -> Result<P, GetParamError> {
        let mut value = MaybeUninit::uninit();
        let ptr = value.as_mut_ptr() as *mut _;

        unsafe {
            magic_getparam(P::KIND, ptr);
        }

        let value = unsafe { value.assume_init() };

        let param = P::from_value(value);
        Ok(param)
    }

    pub fn set_param<P: Param>(&self, param: &P) -> Result<(), SetParamError> {
        let ptr = param.as_ptr() as *const _;

        unsafe {
            magic_setparam(P::KIND, ptr);
        }

        Ok(())
    }
}

let indir_max = cookie.get_param::<IndirMax>()?;

cookie.set_param(&IndirMax(42))?;

[robo9k]

Hej @rainmote
In #453 I've implemented the API design from #433 (comment) that I currently prefer and I also found a problem with the error handling in this pull request here

I intend to merge #453 soon, and close your merge request without merging it
Feel free to take a look at the other merge request and comment whether it also works for your use-cases