nagisa / rust_libloading

Bindings around the platform's dynamic library loading primitives with greatly improved memory safety.
https://docs.rs/libloading
ISC License
1.22k stars 100 forks source link

Provide a easy platform independant way of getting the raw pointer from Symbol #153

Closed AlexanderSchuetz97 closed 1 month ago

AlexanderSchuetz97 commented 2 months ago

Unless I am blind, which I may be, the only way to get the raw pointer appears to be to do the following:

unsafe fn symbol_to_ptr<T>(symbol: libloading::Symbol<T>) -> *mut c_void {
    #[cfg(target_os = "windows")]
    {
        return symbol.into_raw().into_raw().unwrap() as *mut c_void;
    }

    #[cfg(not(target_os = "windows"))]
    {
        return symbol.into_raw().into_raw();
    }
}

I require the c_void in order to call another external c function that needs the pointer. It would be awfully convenient if this function (or a better function that achieves the same thing) was a part of Symbol directly.

Something like symbol.as_raw_ptr() would be amazing!

nagisa commented 2 months ago

I suspect that not every platform is going to be able to provide a callable raw pointer, unfortunately, which makes this symbol_to_ptr intrinsically non-portable. Some potential future targets that come to mind are wasm32-wasi which would instead only allow providing a handle to a table of some sort, platforms like CHERI, etc. Basically platforms where provenance must be retained or the type of callable things isn't pointer at all.

While libloading hasn't improved in terms of platform support past unix+windows since its inception, I would rather not slam that door shut entirely with a convenience function like this.

AlexanderSchuetz97 commented 2 months ago

You could only provide this function for targets that support it or return an Option that would yield None for what you describe.

Ive just looked at my code again i think if you made it so that I could call symbol.into_raw().as_ptr() that would already be sufficient. symbol.into_raw would already return something platform specific. The hypothetical wasi one would just not have this fn and therefore not compile. The main thing id like to avoid is the cfg-if switch in my code. And yeah I dont ever intend to support wasi with my stuff...

nagisa commented 2 months ago

So, the Option from into_raw on Windows is really coming from the way FARPROC was defined in windows-sys where from the definition was copied from. Option<fn ...> is effectively the same nullable pointer as *mut _ in every way, so we could change FARPROC back to a plain pointer, or do the reverse on unix, but in either case its a breaking change.

Is it worth releasing a new breaking release over such an API change? Hard to argue for it in my mind.

On the other hand, we can add as_ptr now and then remove it again whenever there is a breaking change while at the same adjusting into_raw to match between platforms. So I would take a PR that adds such an as_ptr for the time being.

AlexanderSchuetz97 commented 2 months ago

I have made the PR. What we are discussing here is essentially a prime candidate for a TryFrom/TryInto implementation. Ive gone ahead and implement what we discussed using those traits. On platforms that cannot convert to a raw pointer try_into would just fail. As mentioned currently all implements platforms support this so it never fails.