[ping] RFR(M): 8186072: dll_build_name returns true even if file is missing.

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Mon Aug 28 10:10:19 UTC 2017


Hi,

this are the changes needed to make the windows dll_locate_lib 
universally applicable. I also merge the three similar jio_snprintf 
calls into one method.
I do some gymnastics to avoid another buffer of MAX_PATH_LEN
at the first call to conc_path_file_and_check.
I'll test this tonight.

Best regards,
  Goetz.

diff -r e09e4eb985c5 src/os/windows/vm/os_windows.cpp
--- a/src/os/windows/vm/os_windows.cpp	Thu Aug 17 17:26:02 2017 +0200
+++ b/src/os/windows/vm/os_windows.cpp	Mon Aug 28 12:02:26 2017 +0200
@@ -1205,6 +1205,17 @@
   return GetFileAttributes(filename) != INVALID_FILE_ATTRIBUTES;
 }
 
+bool conc_path_file_and_check(char *buffer, char *printbuffer, size_t printbuflen,
+                              const char* pname, char lastchar, const char* fname) {
+  char *filesep = (WINDOWS_ONLY(lastchar == ':' ||) lastchar == os::file_seperator()[0]) ? "" : os::file_separator();
+  int ret = jio_snprintf(printbuffer, printbuflen, "%s%s%s", path, filesep, fullfname);
+  if (ret != -1) {
+    struct stat statbuf;
+    return os::stat(buffer, &statbuf) == 0;
+  }
+  return false;
+}
+
 bool os::dll_locate_lib(char *buffer, size_t buflen,
                         const char* pname, const char* fname) {
   bool retval = false;
@@ -1220,11 +1231,8 @@
       if (p != NULL) {
         const size_t plen = strlen(buffer);
         const char lastchar = buffer[plen - 1];
-        char *filesep = (lastchar == ':' || lastchar == '\\') ? "" : "\\";
-        int ret = jio_snprintf(&buffer[plen], buflen - plen, "%s%s", filesep, fullfname);
-        if (ret != -1) {
-          retval = file_exists(buffer);
-        }
+        retval = conc_path_file_and_check(buffer, &buffer[plen], buflen - plen,
+                                          "", lastchar, fullfname);
       }
     } else if (strchr(pname, *os::path_separator()) != NULL) {
       int n;
@@ -1238,12 +1246,8 @@
             continue; // skip the empty path values
           }
           const char lastchar = path[plen - 1];
-          char *filesep = (lastchar == ':' || lastchar == '\\') ? "" : "\\";
-          int ret = jio_snprintf(buffer, buflen, "%s%s%s", path, filesep, fullfname);
-          if (ret != -1 && file_exists(buffer)) {
-            retval = true;
-            break;
-          }
+          retval = conc_path_file_and_check(buffer, buffer, buflen, path, lastchar, fullfname);
+          if (retval) break;
         }
         // release the storage
         for (int i = 0; i < n; i++) {
@@ -1255,11 +1259,7 @@
       }
     } else {
       const char lastchar = pname[pnamelen-1];
-      char *filesep = (lastchar == ':' || lastchar == '\\') ? "" : "\\";
-      int ret = jio_snprintf(buffer, buflen, "%s%s%s", pname, filesep, fullfname);
-      if (ret != -1) {
-        retval = file_exists(buffer);
-      }
+      retval = conc_path_file_and_check(buffer, buffer, buflen, path, lastchar, fullfname);
     }
   }


> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Montag, 28. August 2017 07:38
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Cc: hotspot-runtime-dev at openjdk.java.net
> Subject: Re: [ping] RFR(M): 8186072: dll_build_name returns true even if file
> is missing.
> 
> Hi Goetz,
> 
> On 25/08/2017 12:19 AM, Lindenmaier, Goetz wrote:
> > Hi,
> >
> > I please need a second review and a sponsor:
> > http://cr.openjdk.java.net/~goetz/wr17/8186072-dllBuildName/webrev.04
> >
> > To update my description of the change to the status after Thomas' review:
> >
> > dll_build_name builds the proper path to a library given a list of paths
> separated by
> > path_seperator and a library name. It adds in the platform specific endings
> etc.
> > It is documented to return whether the file exists, but only does so if a
> path_seperator
> > exists in the path.
> > Especially if the path is empty, it just returns ‘true’ without checking.
> >
> > Dll_build_name is usually used before calling dll_load.  If dll_load does not
> get a full path it searches
> > in well known unix/windows locations. This is intended in the two cases
> where dll_build_name
> > is called with an empty path.
> >
> > I renamed dll_build_name to dll_locate_lib and changed it's behavior to
> always return
> > a full path to the lib, inserting current working directory if no path is given.
> > For the use case where "" was actually passed to the function, I added a
> new function
> > (reusing the old function name) dll_build_name that just adds system
> dependent prefix and suffix
> > to the name.
> > I merged all unix implementations to the posix os branch.
> 
> I started to look at this and have applied the patch to run through some
> basic testing. The overall approach seems reasonable. But it is hard to
> track all the details - in particular whether there were any subtle
> differences across the "posix" systems?
> 
> I'm wondering what, if any, significant differences exist between the
> Windows and POSIX versions? I would hope the platform differences could
> easily be hidden behind macros (for path separator, library suffix etc).
> Then perhaps this could just go in shared code (os.hpp, os.cpp)?
> 
> That aside, in the Windows code shouldn't the hardwired .dll strings
> actually be JNI_LIB_SUFFIX?
> 
> Thanks,
> David
> 
> > Best regards,
> >    Goetz.
> >
> >
> >
> >> -----Original Message-----
> >> From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
> >> Sent: Dienstag, 22. August 2017 17:30
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> >> Cc: hotspot-runtime-dev at openjdk.java.net
> >> Subject: Re: RFR(M): 8186072: dll_build_name returns true even if file is
> >> missing.
> >>
> >> Looks good.
> >>
> >> ..Thomas
> >>
> >> On Tue, Aug 22, 2017 at 4:33 PM, Lindenmaier, Goetz
> >> <goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com> >
> wrote:
> >>
> >>
> >> 	I mistyped the path to webrev, this should work:
> >> 	http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> dllBuildName/webrev.04
> <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> dllBuildName/webrev.04>
> >>
> >> 	Sorry,
> >> 	  Goetz
> >>
> >>
> >>
> >> 	> -----Original Message-----
> >> 	> From: Lindenmaier, Goetz
> >> 	> Sent: Dienstag, 22. August 2017 15:48
> >> 	> To: 'Thomas Stüfe' <thomas.stuefe at gmail.com
> >> <mailto:thomas.stuefe at gmail.com> >
> >> 	> Cc: hotspot-runtime-dev at openjdk.java.net <mailto:hotspot-
> runtime-
> >> dev at openjdk.java.net>
> >> 	> Subject: RE: RFR(M): 8186072: dll_build_name returns true even if
> >> file is
> >> 	> missing.
> >> 	>
> >> 	> Hi,
> >> 	>
> >> 	> could I please get a second review?
> >> 	> http://cr.openjdk.java.net/~goetz/wr17/8186072-dllBuildName-
> >> hs/webrev.04 <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> dllBuildName-
> >> hs/webrev.04>
> >> 	>
> >> 	> I had to update the webrev because of a problem on windows.
> >> 	> @Thomas I had edited os.hpp, but not saved :(
> >> 	>
> >> 	> Best regards,
> >> 	>   Goetz.
> >> 	>
> >> 	> PS: Didn't double-check the webrev as cr server is slow.
> >> 	>
> >> 	> > -----Original Message-----
> >> 	> > From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com
> >> <mailto:thomas.stuefe at gmail.com> ]
> >> 	> > Sent: Donnerstag, 17. August 2017 19:54
> >> 	> > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com
> >> <mailto:goetz.lindenmaier at sap.com> >
> >> 	> > Cc: hotspot-runtime-dev at openjdk.java.net <mailto:hotspot-
> >> runtime-dev at openjdk.java.net>
> >> 	> > Subject: Re: RFR(M): 8186072: dll_build_name returns true even
> if
> >> file is
> >> 	> > missing.
> >> 	> >
> >> 	> > Hi Goetz,
> >> 	> >
> >> 	> > On Thu, Aug 17, 2017 at 6:03 PM, Lindenmaier, Goetz
> >> 	> > <goetz.lindenmaier at sap.com
> >> <mailto:goetz.lindenmaier at sap.com>
> <mailto:goetz.lindenmaier at sap.com
> >> <mailto:goetz.lindenmaier at sap.com> > > wrote:
> >> 	> >
> >> 	> >
> >> 	> >     Hi Thomas,
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     I adapted the comments in os.hpp.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     If I move the call to dll_build_name out of dll_locate_lib
> >> 	> >
> >> 	> >     I have to do a lot of coding in all the places where it is called.
> >> 	> >
> >> 	> >     That seems not useful to me.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Fixed the type to size_t.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     One could merge posix/windows if putting the check for ‘:’
> >> 	> >
> >> 	> >     into a WINDOWS_ONLY() I guess. The check for \ could be
> >> 	> >
> >> 	> >     done in posix as well, if using file_seperator().
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     *  Not your change, but: why does the code in
> os::dll_locate_lib()
> >> even
> >> 	> >
> >> 	> >     *  differentiate between a PATH containing no
> >> os::path_separator()
> >> 	> >
> >> 	> >     *  and a path containing os::path_separator()?
> >> 	> >
> >> 	> >     I assume this was done to avoid all the allocations and copying
> of
> >> the
> >> 	> > path.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Also adapted the comment in jvmtiExport.cpp.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     New webrev:
> >> 	> >
> >> 	> >     http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> > dllBuildName/webrev.03/
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> > dllBuildName/webrev.03/>
> >> 	> >
> >> 	> >     incremental diff:
> >> 	> >
> >> 	> >     http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> > dllBuildName/webrev.03/diffs-incremental.patch
> >> 	> > <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> > dllBuildName/webrev.03/diffs-incremental.patch>
> >> 	> >
> >> 	> >     (fixed indentation on windows)
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Best regards,
> >> 	> >
> >> 	> >       Goetz.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> > Comments in os.hpp seem unchanged ?
> >> 	> >
> >> 	> > But looks fine otherwise. I do not need another webrev.
> >> 	> >
> >> 	> > Thanks, Thomas
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com
> >> <mailto:thomas.stuefe at gmail.com>
> >> 	> > <mailto:thomas.stuefe at gmail.com
> >> <mailto:thomas.stuefe at gmail.com> > ]
> >> 	> >     Sent: Thursday, August 17, 2017 3:48 PM
> >> 	> >     To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com
> >> <mailto:goetz.lindenmaier at sap.com>
> >> 	> > <mailto:goetz.lindenmaier at sap.com
> >> <mailto:goetz.lindenmaier at sap.com> > >
> >> 	> >     Cc: hotspot-runtime-dev at openjdk.java.net <mailto:hotspot-
> >> runtime-dev at openjdk.java.net>  <mailto:hotspot-runtime-
> <mailto:hotspot-
> >> runtime->
> >> 	> > dev at openjdk.java.net <mailto:dev at openjdk.java.net> >
> >> 	> >     Subject: Re: RFR(M): 8186072: dll_build_name returns true
> even
> >> if file
> >> 	> > is missing.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Hi Goetz,
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     On Thu, Aug 17, 2017 at 1:35 PM, Lindenmaier, Goetz
> >> 	> > <goetz.lindenmaier at sap.com
> >> <mailto:goetz.lindenmaier at sap.com>
> <mailto:goetz.lindenmaier at sap.com
> >> <mailto:goetz.lindenmaier at sap.com> > > wrote:
> >> 	> >
> >> 	> >             Hi Thomas,
> >> 	> >
> >> 	> >             I reworked the whole thing.
> >> 	> >
> >> 	> >             First, there is dll_build_name. It just does <name> ->
> >> 	> > lib<name>.so.
> >> 	> >
> >> 	> >             Second, I renamed the legacy dll_build_name to
> >> dll_locate_lib.
> >> 	> >
> >> 	> >             I merged all the unix variants to one in os_posix.
> >> 	> >
> >> 	> >             I removed the buffer overflow check at the top.
> >> 	> >             It's too restrictive because the path argument
> >> 	> >             can contain several paths.  I added the overflow
> >> 	> >             checks into the single cases.
> >> 	> >
> >> 	> >             Also, I first assemble the pure name using the new, simple
> >> 	> >             dll_build_name. This is for reuse and readability.
> >> 	> >
> >> 	> >             In case of an empty directory, I use get_current_directory
> >> 	> >             to complete the path as indicated by the original
> >> 	> > documentation
> >> 	> >             where it was called with "".
> >> 	> >             Dll_locate_lib now always returns a name with a full path if
> >> 	> >             the file exists.
> >> 	> >
> >> 	> >             Also, on windows, I think I fixed a bug by reversing the
> order
> >> 	> >             of checks. A path list ending in ':' or '\' would not have
> >> 	> >             been recognized.
> >> 	> >
> >> 	> >             On Bsd, I removed JNI_LIB_* because that already is
> defined
> >> 	> >             in jvm_bsh.h
> >> 	> >
> >> 	> >             New webrev:
> >> 	> >             http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> > dllBuildName/webrev.02/
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> > dllBuildName/webrev.02/>
> >> 	> >
> >> 	> >             Best regards,
> >> 	> >               Goetz.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     I like this better than before. Remarks:
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> >
> dllBuildName/webrev.02/src/share/vm/runtime/os.hpp.udiff.html
> >> 	> > <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> >
> dllBuildName/webrev.02/src/share/vm/runtime/os.hpp.udiff.html>
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     +  // Builds the platform-specific name of a library.
> >> 	> >
> >> 	> >     +  // Returns false on __buffer overflow__.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Hopefully not! :D
> >> 	> >
> >> 	> >     How about: "Returns false no truncation" instead.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     +  // Builds a platform-specific full library path given an ld path
> >> and lib
> >> 	> > name.
> >> 	> >
> >> 	> >     +  // Returns true if the buffer contains a full path to an existing
> >> file,
> >> 	> > false
> >> 	> >
> >> 	> >     +  // otherwise. If pathname is empty, checks the current
> >> directory.
> >> 	> >
> >> 	> >     +  static bool           dll_locate_lib(char* buffer, size_t size,
> >> 	> >
> >> 	> >                                             const char* pathname, const char*
> >> fname);
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Might be worth mentioning that "fname" is the unadorned
> library
> >> 	> > name, e.g. "verify" for libverify.so or verify.dll.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Would the following alternative be valid:
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     one could make dll_locate_lib take the real file name, and let
> >> caller
> >> 	> > use dll_build_name() to build the libary name first before handing
> it
> >> to
> >> 	> > dll_locate_lib(). In that case, dll_locate_lib() could be renamed to
> a
> >> generic
> >> 	> > "find_file_in_path" because it would work for any kind of file.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     As an added bonus, there would be no need to create a
> >> temporary
> >> 	> > array in dll_build_name/dll_locate_lib, and no need to call free()
> so
> >> no
> >> 	> > cleanup-related control flow changes in these functions.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     =====
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> >
> >>
> dllBuildName/webrev.02/src/os/windows/vm/os_windows.cpp.udiff.html
> >> 	> > <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> >
> >>
> dllBuildName/webrev.02/src/os/windows/vm/os_windows.cpp.udiff.html>
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     +  int fullfnamelen = strlen(JNI_LIB_PREFIX) + strlen(fname) +
> >> 	> > strlen(JNI_LIB_SUFFIX);
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     int -> size_t (does that even compile without warning?)
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     +    // Check current working directory.
> >> 	> >
> >> 	> >     +    const char* p = get_current_directory(buffer, buflen);
> >> 	> >
> >> 	> >     +    if (p != NULL &&
> >> 	> >
> >> 	> >     +        strlen(buffer) + 1 + fullfnamelen + 1 <= buflen) {
> >> 	> >
> >> 	> >     +      strcat(buffer, "\\");
> >> 	> >
> >> 	> >     +      strcat(buffer, fullfname);
> >> 	> >
> >> 	> >     +      retval = file_exists(buffer);
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Small nit: I'd use jio_snprintf instead of strcat. Functionally
> >> identical but
> >> 	> > will make scanners (e.g. coverity) happy. One could then avoid
> the
> >> length
> >> 	> > calculation and rely on jio_snprintf truncation:
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     const char* p = get_current_directory(buffer, buflen);
> >> 	> >
> >> 	> >     if (p != NULL) {
> >> 	> >
> >> 	> >       const size_t end = strlen(p);
> >> 	> >
> >> 	> >       if (jio_snprintf(end, buflen - end, "\\%s", fullname) != -1) {
> >> 	> >
> >> 	> >         retval = file_exists(buffer);
> >> 	> >
> >> 	> >       }
> >> 	> >
> >> 	> >     }
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     --
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Not your change, but: why does the code in os::dll_locate_lib()
> >> even
> >> 	> > differentiate between a PATH containing no os::path_separator()
> >> and a path
> >> 	> > containing os::path_separator()?
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Would the former not be just a PATH with only one directory
> and
> >> hence
> >> 	> > need no special treatment?
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     =====
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> >
> dllBuildName/webrev.02/src/os/posix/vm/os_posix.cpp.udiff.html
> >> 	> > <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> >
> dllBuildName/webrev.02/src/os/posix/vm/os_posix.cpp.udiff.html>
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Could os::dll_locate_lib be consolidated between windows and
> >> unix?
> >> 	> > Seems to be the implementation is almost identical.
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     ====
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> >
> >> dllBuildName/webrev.02/src/share/vm/prims/jvmtiExport.cpp.udiff.html
> >> 	> > <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> >
> >>
> dllBuildName/webrev.02/src/share/vm/prims/jvmtiExport.cpp.udiff.html>
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     +        // not found - try library path
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Proposal: "not found - try OS default library path"
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >             Find some comments inline:
> >> 	> >
> >> 	> >
> >> 	> >             >       Especially if the path is empty, it just returns 'true'.
> >> 	> >             >       Dll_build_name is usually used before calling dll_load.
> >> If
> >> 	> > dll_load does not get a full path it searches
> >> 	> >             >       in well known unix/windows locations. This is intended
> >> in
> >> 	> > the two cases where dll_build_name
> >> 	> >             >       is called with an empty path.
> >> 	> >             >
> >> 	> >             > So, for both cases (thread.cpp, jvmtiExport.cpp),
> >> 	> >             >
> >> 	> >             > before, we would call os::dll_build_name() with an empty
> >> 	> > string for the path
> >> 	> >             > which, for relative paths, would result in feeding that path
> >> 	> > unexpanded to
> >> 	> >             > dlopen(), which would use whatever the OS does in those
> >> 	> > cases (LIBPATH,
> >> 	> >             > LD_LIBRARY_PATH, PATH on windows). Note that this
> does
> >> 	> > not necessarily
> >> 	> >             > include searching the current directory.
> >> 	> >             Right. With changed dll_biuld_name it's again exactly as
> >> 	> > before.
> >> 	> >
> >> 	> >             > With your change, we now use java.library.path, which is
> >> not
> >> 	> > necessarily the
> >> 	> >             > same?
> >> 	> >             You are right, I oversaw that java.library.path can be
> >> 	> > overwritten.  Initially,
> >> 	> >             it's set to the right thing.
> >> 	> >
> >> 	> >             > (BTW, I think the old comments in thread.cpp and
> >> 	> > jniExport.cpp were wrong:"//
> >> 	> >             > Try the local directory" - if "local" means "current", this is
> >> not
> >> 	> > what did
> >> 	> >             > happen).
> >> 	> >             Right, I tried to adapt them, did I miss one?
> >> 	> >
> >> 	> >             >       I added a second variant of dll_build_name without
> the
> >> 	> > path argument that adds the path
> >> 	> >             >       from system property java.lang.path and use that in
> >> these
> >> 	> > two cases.
> >> 	> >             >       I changed the original function to actually check file
> >> 	> > availability in all cases,
> >> 	> >             >       and to check . if the path is empty.
> >> 	> >             > I think that may be a bit confusing. We would then have
> >> three
> >> 	> > options:
> >> 	> >             >
> >> 	> >             > - call os::dll_build_name with a real "<aa>;<bb>;.." PATH
> >> and
> >> 	> > get a file name
> >> 	> >             > resolved from that path
> >> 	> >             > - call os::dll_build_name with "" for the PATH and get OS
> >> dll
> >> 	> > resolution
> >> 	> >             No, in that case, as I called file_exists(), it would only work if
> >> 	> > the dll is in the
> >> 	> >             current working directory. But I changed this now, anyways.
> >> 	> >
> >> 	> >             > - call your new overloaded version of
> os::dll_build_name(),
> >> 	> > which uses -
> >> 	> >             > Djava.library.path.
> >> 	> >             >
> >> 	> >             >       Please review this change. I please need a sponsor.
> >> 	> >             >       http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> > <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072-> >
> >> 	> >             > dllBuildName/webrev.01/
> >> 	> > <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072->
> >> 	> > <http://cr.openjdk.java.net/~goetz/wr17/8186072-
> >> <http://cr.openjdk.java.net/~goetz/wr17/8186072-> >
> >> 	> >             > dllBuildName/webrev.01/>
> >> 	> >
> >> 	> >             >
> >> 	> >             >       Best regards,
> >> 	> >             >         Goetz.
> >> 	> >             >
> >> 	> >             >
> >> 	> >             >
> >> 	> >             >
> >> 	> >             > Kind Regards, Thomas
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >     Best Regards, Thomas
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >> 	> >
> >>
> >>
> >>
> >


More information about the hotspot-runtime-dev mailing list