mjansson / foundation_lib

Cross-platform public domain foundation library in C providing basic support data types and functions to write applications and games in a platform-independent fashion.
The Unlicense
296 stars 24 forks source link

Loading library from relative directory doesn't work #9

Closed emoon closed 9 years ago

emoon commented 9 years ago

When using

library_load("my_path/some_lib");

The code assumes that there is no path in the in the name being sent in. The code will expand the string to look like this (on Mac)

"libmy_path/some_lib.dylib"

Which means that the current code always adds "lib" at the start regardless if there is a directory there or not.

emoon commented 9 years ago

Maybe add another function called library_load_fullname/fullpath or something like that where the user has to provide a fully formated path instead.

Of course it's your library so you do as you wish of course :)

mjansson commented 9 years ago

Good catch, that particular code is an migration from another codebase and some legacy got included. Will clean it up.

emoon commented 9 years ago

Thanks :)

mjansson commented 9 years ago

I've cleaned up the fallback, which makes sense in that the idea is to be able to call library_load( "foo" ) without having to worry about platform specific prefix/suffix.

The fallback now only triggers if the platform extension is not present, and the fallback adds the prefix/suffix on the file name, preserving paths. It always tries to load the given name first, unmodified.

Check out https://github.com/rampantpixels/foundation_lib/tree/feature/library-load and see if you agree it's a reasonable implementation

emoon commented 9 years ago

Looks good. Something that might be worth considering checking is for backslashes in the path here (on Windows) https://github.com/rampantpixels/foundation_lib/blob/feature/library-load/foundation/library.c#L100

I really don't use that myself but I can see that other people will.

mjansson commented 9 years ago

Merged in 8194e3babb

emoon commented 9 years ago

Tested this now and works as expected. Thanks!

Might be good to add some more tests to validate the code (as you already have really good tests for most of the code)