Closed Bug 511315 Opened 11 years ago Closed 10 years ago
_Load Library With Flags should have a way to set LOAD _WITH _ALTERED _SEARCH _PATH flag with Load Library Ex
PR_LoadLibraryWithFlags currently uses LoadLibrary. It should be switched to using LoadLibraryEx so that it can take advantage of the LOAD_WITH_ALTERED_SEARCH_PATH flag . This would allow smoother loading of the direct-linked dependencies of DLLs. For example, this flag allows the following case to work : If the executable is running from C:\A.EXE , it can load a C:\DIR\B.DLL with a dependency of C:\DIR\C.DLL even if C:\DIR isn't in the PATH .
For more info about this flag, see http://msdn.microsoft.com/en-us/library/ms682586(VS.85).aspx . Quote : "The LoadLibraryEx function supports an alternate search order if the call specifies LOAD_WITH_ALTERED_SEARCH_PATH and the lpFileName parameter specifies an absolute path. Note that the standard search strategy and the alternate search strategy specified by LoadLibraryEx with LOAD_WITH_ALTERED_SEARCH_PATH differ in just one way: The standard search begins in the calling application's directory, and the alternate search begins in the directory of the executable module that LoadLibraryEx is loading." The flag does not work with relative PATHs, per http://msdn.microsoft.com/en-us/library/ms684179(VS.85).aspx . I think we cannot set this flag unconditionally because of binary compatibility implications. We need to create a new flag for PR_LoadLibraryWithFlags to trigger this system flag for LoadLibraryEx.
No longer blocks: 511312
Wan-Teh, Please review. Even if we don't immediately decide to use this to resolve bug 511312, this is probably a good thing to have in NSPR.
Attachment #395494 - Flags: review?(wtc)
What's the abstraction here? Seems to me that NSPR should attempt to provide a virtual platform, with platform-independent features, offered on all supported platforms, insofar as possible. So, what might the equivalent functionality be for PR_LD_ALTERED_SEARCH_PATH on Linux or MacOSX?
Comment on attachment 395494 [details] [diff] [review] Add PR_LD_ALTERED_SEARCH_PATH flag r=wtc. Please attach a new patch with the following changes before you check it in. >+#define PR_LD_ALTERED_SEARCH_PATH 0x10 /* equivalent to >+ * LOAD_WITH_ALTERED_SEARCH_PATH on Windows Please shorten this to PR_LD_ALT_SEARCH_PATH. I wonder if it would look nicer to move the comment to the previous line, like this: /* equivalent to LOAD_WITH_ALTERED_SEARCH_PATH on Windows */ #define PR_LD_ALT_SEARCH_PATH 0x10 >+ h = LoadLibraryExW(wname, NULL, (flags & PR_LD_ALTERED_SEARCH_PATH) ? >+ LOAD_WITH_ALTERED_SEARCH_PATH : 0); Please move the third argument to the second line: h = LoadLibraryExW(wname, NULL, (flags & PR_LD_ALT_SEARCH_PATH) ? LOAD_WITH_ALTERED_SEARCH_PATH : 0); You also need to add this function to mozilla/nsprpub/pr/src/nspr.def. Add a new NSPR_4.8.1 section.
Comment on attachment 395494 [details] [diff] [review] Add PR_LD_ALTERED_SEARCH_PATH flag Ah, nspr.def doesn't need to be changed because you didn't add a new function. Sorry about the confusion. To address Nelson's comment, we can make it clear that this is a Windows-only feature: #ifdef WIN32 /* equivalent to LOAD_WITH_ALTERED_SEARCH_PATH on Windows */ #define PR_LD_ALT_SEARCH_PATH 0x10 #endif or without the ifdef: /* * equivalent to LOAD_WITH_ALTERED_SEARCH_PATH on Windows, * ignored on other platforms */ #define PR_LD_ALT_SEARCH_PATH 0x10 Another option is to make NSPR try the alternate search path automatically if the standard search path doesn't work. This has the drawback that the failure of PR_LoadLibrary will be twice as expensive. Some apps call PR_LoadLibrary to try a few possible locations of a DLL, so some of those PR_LoadLibrary calls are expected to fail, and this will hurt them. As I noted in bug 454508 comment 6, we can't tell by the error code of the first LoadLibrary call whether retrying with alternate search path will help, so we will always need to try it.
Nelson, re: comment 3, There is no abstraction in this case, it is just wrapping of platform-specific features. If you look at the other pre-existing PR_LD_ flags, they are platform-specific too, and apply to Unix only. My patch just followed the existing trend in adding one more platform-specific flag, but this time for Windows. This new flag is simply ignored on non-Windows platforms, just as these other pre-existing flags are ignored on non-Unix platforms. Wan-Teh, re: comment 4, I can fix the indentation & comments as well as shorten the flag name. No change to nspr.def is necessary. re: comment 5, I think it's undesirable to have this alternate search path as the default behavior. It is likely to cause undesired effects in apps that depend on the old behavior. This is why I implemented this feature as an optional flag, and did not try to make the alternate search automatic.
Checking in pr/include/prlink.h; /cvsroot/mozilla/nsprpub/pr/include/prlink.h,v <-- prlink.h new revision: 3.16; previous revision: 3.15 done Checking in pr/src/linking/prlink.c; /cvsroot/mozilla/nsprpub/pr/src/linking/prlink.c,v <-- prlink.c new revision: 3.103; previous revision: 3.102 done
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.