Rename Weak to LazyPtr and move it to lazy.rs

[newpavlov]

Currently the Weak type is used only by the NetBSD backend, but in future it may be used by other backends. To simplify code a bit and prepare for potential use on Windows, Weak now accepts a "pointer initialization function" instead of a function name, i.e. it now works similarly to LazyUsize and LazyBool.

[briansmith]

I think we should have a plan for #285 first. It may be that we start using Weak on Linux too?

Also, I think we should sort out what we're doing for Windows, as we're basically implementing Weak using the Windows APIs in #415. Although PR #415 isn't written as an implementation of Weak for Windows, I think we should do that, if we're going with the LoadLibrary(Ex)-based approach, because the sandboxing concerns (at least) are analogous across platforms.

[josephlr]

Agreed with @briansmith here, lets figure out if we need Weak on other targets first.

[newpavlov]

I have changed this PR and now Weak resides in a separate module. The new Weak now uses linking function pointer instead of linked function name. It should address @briansmith's concerns which he tried to resolve in #426 and make its code a bit more future-proof.

[josephlr]

Looks good, just a few nits.

[newpavlov]

@josephlr
I've tweaked the code a bit and moved linking function to the get method (previously ptr). I think it's a slightly better approach since we do not need redundant store of the function pointer. Theoretically this API could be misused by calling get with different functions, but since it's a private API, it should not cause any issues.

[briansmith]

It would be good for somebody to actually test this and any future refactorings of Weak on a NetBSD machine.

[josephlr]

I like this implementation. One final question, should this now live in lazy.rs as its now quite similar to LazyUsize. If we do decide to move it, we should probably rename to LazyPtr.

Regarding @briansmith's comment:

It would be good for somebody to actually test this and any future refactorings of Weak on a NetBSD machine.

Could we add a test that invokes this on Linux? Might be better than nothing. Could look like a tests/linux_dlsym.rs

// Only test on Linux/Android
#![cfg(any(target_os = "linux", target_os = "android"))]

#[path = "../src/lazy.rs"]
mod lazy;
use lazy::LazyPtr;

static GETRANDOM: LazyPtr = LazyPtr::new();

fn dlsym_getrandom() -> *mut c_void {
    static NAME: &[u8] = b"getrandom\0";
    let name_ptr = NAME.as_ptr() as *const libc::c_char;
    unsafe { libc::dlsym(libc::RTLD_DEFAULT, name_ptr) }
}

fn getrandom_impl(dest: &mut [u8]) -> Result<(), Error> {
    let fptr = GETRANDOM.get(dlsym_getrandom).unwrap();
    ...
}
mod common;

No need to add it if you don't think the test is maintainable or its too much work.

[newpavlov]

One final question, should this now live in lazy.rs as its now quite similar to LazyUsize. If we do decide to move it, we should probably rename to LazyPtr.

Good observation! Although there is a certain difference between LazyUsize and LazyPtr in the orderings used under the hood.

[briansmith]

I do think having the test run on --linux-gnu (where we know dlsym makes sense) is a good idea.

let fptr = GETRANDOM.get(dlsym_getrandom).unwrap();
...

Note that we should actually call the function through fptr. We should have LazyPtr::get() do the pointer cast transmute at the end so that the test doesn't need to repeat it.

[briansmith]

I think we should have a plan for #285 first. It may be that we start using Weak on Linux too?

Also, I think we should sort out what we're doing for Windows,

It seems like for both of those, we're close to deciding now to use LazyPtr. If so, maybe we don't need to make this change, unless we want to make a change to improve the testing.

[newpavlov]

maybe we don't need to make this change, unless we want to make a change to improve the testing.

The result is cleaner and this approach does not need the CStr ersatz implemented in your other PR.

I do think having the test run on --linux-gnu (where we know dlsym makes sense) is a good idea.

I would prefer to have a proper VM-based testing implemented in a separate PR.

[josephlr]

Thanks!!

[josephlr]

I would prefer to have a proper VM-based testing implemented in a separate PR.

Sounds good to me.