Improve shared library relative paths handling #320
Conversation
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
src/shared_library.c
Outdated
| image_name = candidate_name; | ||
| } | ||
| if (dlclose(handle) != 0) { | ||
| RCUTILS_SET_ERROR_MSG("dlclose error: %s", dlerror()); |
There was a problem hiding this comment.
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
hidmic
requested review from
azeey and
ahcorde
and removed request for
clalancette and
ahcorde
| #if defined(__APPLE__) | ||
| const char * image_name = NULL; | ||
| uint32_t image_count = _dyld_image_count(); | ||
| for (uint32_t i = 0; NULL == image_name && i < image_count; ++i) { |
There was a problem hiding this comment.
I feel like I'm missing something. Is my understanding correct that what ends up in lib->library_path is the absolute path of library_path? If so, can't we just call realpath? Or is it possible library_path and lib->library_path will point to two different files?
There was a problem hiding this comment.
If so, can't we just call
realpath?
The thing is, library_path might simply be the library name e.g. libfoo.so. And that doesn't mean it should be searched in the current working directory. It's only after dlopen performs the search over RPATHs, (DY)LD_LIBRARY_PATHs, and RUNPATHs and actually finds it that you can get the full path to it.
There was a problem hiding this comment.
Ah, that makes sense. Thanks! I don't know what the comment etiquette is for this package, but I think it would be great to have that paragraph as a comment just to explain what the goal of this section of code is.
There was a problem hiding this comment.
Sure thing. See 1d4276a.
| #if defined(__APPLE__) | ||
| const char * image_name = NULL; | ||
| uint32_t image_count = _dyld_image_count(); | ||
| for (uint32_t i = 0; NULL == image_name && i < image_count; ++i) { |
There was a problem hiding this comment.
Ah, that makes sense. Thanks! I don't know what the comment etiquette is for this package, but I think it would be great to have that paragraph as a comment just to explain what the goal of this section of code is.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
Ok, I finally managed to write some tests! It stumbled with |
|
Re-running CI:
|
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
Alright, all tests passing! Thanks for the reviews. |
|
@hidmic, this change broke clang nightly builds on macOS. Please submit a fix. |
Connected to #143. This patch ensures relative library paths are properly handled.
CI up to
test_communication: