added dynamic library loading via libdl #2598
Conversation
|
The |
|
doesn't |
|
@emekoi where did you land with this? If it's ready for merge, would you mind rebasing it and let's try to get that CI light to turn green |
|
i will try to clean this up and rebase it by the end of the week. |
|
can anyone think of a way to solve #2435? or would passing a |
std/dynamic_library.zig
Outdated
| pub fn lookup(self: *WindowsDynLib, name: []const u8) ?usize { | ||
| if (windows.kernel32.GetProcAddress(self.dll, name.ptr)) |addr| { | ||
| return @ptrToInt(addr); | ||
| pub fn lookup(self: *WindowsDynLib, comptime T: type, name: []const u8) !?T { |
There was a problem hiding this comment.
It's unfortunate that this adds the possibility of failure. Maybe there can be lookupC (following the patterns in std.fs.File) which accepts a null_terminated_name: [*]const u8 (this can be changed once #265 is done), and has no possibility of failure. lookup would do this null termination and then call lookupC. Then API users can opt in to using C string literals and have no possibility of failure. (And again after #265 normal string literals would also work.)
Another reasonable way to design this API would be to have lookup accept a comptime max_name_len: usize and use that as the c_name array size. The documentation for the function would say, caller guarantees that name.len <= max_name_len. This pushes the only possible error condition out of lookup and onto the caller.
There was a problem hiding this comment.
so if the caller passes an array that's too small they get a buffer overflow?
There was a problem hiding this comment.
Yes in the API I'm proposing, if they don't fulfill the guarantee, it would be undefined behavior in release-small & release-fast, and would trigger runtime safety in debug & release-safe (either in the call to std.mem.copy or the part where it sets the zero). It would be pretty clear at the callsite, because they would be consciously choosing this max_name_len.
There was a problem hiding this comment.
what about LinuxDynLib? will the lookup function change based on the target?
There was a problem hiding this comment.
Linux dyn lib needs a check for whether we're linking libc or not. If we're linking libc then it's the same API as darwin, I believe. So a cross platform application is limited to this API anyway. The more restrictive API can work on all platforms. However, we can also provide the nicer API for Linux that takes a slice and doesn't fail, and it will be documented as being available under certain conditions, and when those conditions are not met, it will be a compile error. So the programmer has access to a cross platform API which is restricted to the lowest common denominator, as well as target-specific API(s) that have different capabilities but using them means giving up portability. With Zig's comptime logic, we can provide the programmer with all of these options. The path of least resistance should be the cross-platform API.
There was a problem hiding this comment.
Also to be clear I don't expect you to necessarily do all of this in this PR, I'm just trying to give you the vision.
emekoi
changed the title
|
@andrewrk thanks for merging this |
i implemented dynamic library loading for darwin using dlsym. getting errors back is going to be somewhat involved as
dlerroronly returns strings. i'm also not sure if we should check to see if the library being loaded is compatible with the loading process usingdlopen_preflight.i also fixed a logic errors in the windows code for loading dll's.
DynLib.lookupwould return 0 instead ofnullif the symbol couldn't be found.