[ping] RFR(M): 8186072: dll_build_name returns true even if file is missing.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Tue Aug 29 06:18:42 UTC 2017
Hi,
this is a webrev with merged windows and posix implementations:
http://cr.openjdk.java.net/~goetz/wr17/8186072-dllBuildName/webrev.05/
Best regards,
Goetz
> -----Original Message-----
> From: Lindenmaier, Goetz
> Sent: Montag, 28. August 2017 12:10
> To: 'David Holmes' <david.holmes at oracle.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,
>
> 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