Thank you, Staffan! Best regards, Kris
On Thu, May 15, 2014 at 11:46 PM, Staffan Larsen <staffan.lar...@oracle.com>wrote: > > On 15 maj 2014, at 22:19, Krystal Mok <rednaxel...@gmail.com> wrote: > > Hi everyone, > > May I have a couple of review for this small patch, please? > > *Bug*: https://bugs.openjdk.java.net/browse/JDK-8043264 > > *Patch*: (against jdk9/hs) > > diff -r 466b58fa837b src/share/vm/compiler/disassembler.cpp > --- a/src/share/vm/compiler/disassembler.cpp Thu May 15 11:35:26 2014 > -0700 > +++ b/src/share/vm/compiler/disassembler.cpp Thu May 15 13:14:58 2014 > -0700 > @@ -86,7 +86,7 @@ > { > // Match "jvm[^/]*" in jvm_path. > const char* base = buf; > - const char* p = strrchr(buf, '/'); > + const char* p = strrchr(buf, *os::file_separator()); > if (p != NULL) lib_offset = p - base + 1; > p = strstr(p ? p : base, "jvm"); > if (p != NULL) jvm_offset = p - base; > @@ -111,7 +111,7 @@ > if (_library == NULL) { > // 3. <home>/jre/lib/<arch>/hsdis-<arch>.so > buf[lib_offset - 1] = '\0'; > - const char* p = strrchr(buf, '/'); > + const char* p = strrchr(buf, *os::file_separator()); > if (p != NULL) { > lib_offset = p - buf + 1; > strcpy(&buf[lib_offset], hsdis_library_name); > > > Looks good! > > Thanks, > /Staffan > > > > *Description*: > > (Copied from the bug) > > On Windows, the hsdis library isn't picked up correctly in all expected > paths described in Disassembler::load_library(): > > // Find the disassembler shared library. > // Search for several paths derived from libjvm, in this order: > // 1. <home>/jre/lib/<arch>/<vm>/libhsdis-<arch>.so (for compatibility) > // 2. <home>/jre/lib/<arch>/<vm>/hsdis-<arch>.so > // 3. <home>/jre/lib/<arch>/hsdis-<arch>.so > // 4. hsdis-<arch>.so (using LD_LIBRARY_PATH) > > The reason is that the code that concatenates the paths doesn't take > os::file_separator() into account, and always uses '/' instead, like: > > const char* p = strrchr(buf, '/'); > > The fix is to change the use of '/' into *os::file_separator() on two > lines in Disassembler::load_library(). > > *Testing* > > The change was tested by a friend of mine on Windows and he said hsdis.dll > could be picked up from expected paths after the change. I don't have a > Windows box to test it myself, so I'd like someone from Oracle to help me > test it. > > P.S. I wanted to upload a webrev, but somehow I couldn't get the the > connection to work. Could it be that my publickey is missing on > cr.openjdk.java.net? > > The authenticity of host 'cr.openjdk.java.net (137.254.56.61)' can't be > established. > RSA key fingerprint is d2:d1:ee:03:4b:11:45:49:ec:46:99:8c:e4:c3:00:c4. > Are you sure you want to continue connecting (yes/no)? yes > Warning: Permanently added 'cr.openjdk.java.net,137.254.56.61' (RSA) to > the list of known hosts. > Permission denied (publickey). > lost connection > > Best regards, > Kris (OpenJDK username: kmo) > > >
