On 11/15/2012 2:49 PM, serguei.spit...@oracle.com wrote:
Bill,
It looks good.
Minor:
The line "int n;" is not needed and can be deleted in the files:
||*src/solaris/back/linker_md.c*
||*src/windows/demo/jvmti/hprof/hprof_md.c*
* src/windows/back/linker_md.c*
*src/windows/demo/jvmti/hprof/hprof_md.c*
||*
*
Oops. Missed that one. Fixed now in webrev.02
**Similar simplification can be done:
* src/windows/demo/jvmti/hprof/hprof_md.c*
Did that but did not save file to disk before running webrev.
I do not have a reviewer status, but you can list as a reviewer. :)
Thank you for fixing it!
Ok, will do.
thanks,
bill
Thanks,
Serguei
On 11/15/12 11:27 AM, BILL PITTORE wrote:
Hi Serguei,
Thanks for the review and your excellent suggestion at some
simplification in dll_build_name. Applied the changes, built, test
and all is still working. New webrev at:
http://cr.openjdk.java.net/~bpittore/7200297/webrev.01/
Are you an official reviewer?
thanks,
bill
On 11/15/2012 4:00 AM, serguei.spit...@oracle.com wrote:
||Hi Bill,
*src/share/back/debugInit.c*
**271 JVMTI_FUNC_PTR(gdata->jvmti,GetSystemProperty)
Space is missed after comma.
||*src/share/back/error_messages.c
*
74 /* May be called before NPT is initialized so don't fault */
The comment seems to belong to the else statement.
||*src/share/back/transport.c
*||*src/share/demo/jvmti/hprof/hprof.h*||*
src/share/demo/jvmti/hprof/hprof_init.c*
No comments.
||*src/solaris/back/linker_md.c
*
***Lines 75-96:*
It seems the *if/else* is not needed as the while loop would
serve the else as well.
* 73 int n;*
Line #73 can be removed as "n" is not used.
* 81 if ((p - pathname) == 0) {*
Nit: Replace *(p - pathname) == 0* with *p == pathname*
* 119 char *p;*
Line #119 can be removed as "p" is not used.
||*src/solaris/demo/jvmti/hprof/hprof_md.c
*
***Lines 395-418:*
The same comments as for the *linker_md.c*.
||*src/solaris/npt/npt_md.h
*
No comments.
||*src/windows/back/linker_md.c
*
The same comments as for the *linker_md.c*.
||*src/windows/demo/jvmti/hprof/hprof_md.c
*
****Lines 381-407:*
*
The same comments as for the *linker_md.c*.
*
*
||*src/windows/npt/npt_md.h
*
No comments.
Thanks,
Serguei
On 11/14/12 2:27 PM, bill.pitt...@oracle.com wrote:
This bug has to do with the jdwp and hprof agents not parsing the
sun.boot.library.path (dll_dir) correctly since the fix for 6819213
went in some years ago. This bug popped up while working on a
particular platform that does not have the ability to set
LD_LIBRARY_PATH before running the VM. As documented in the bug, on
most platforms even if the sun.boot.library.path consists of
multiple paths and the jdwp or hprof code fails to load a dependent
lib, the system falls back to using LD_LIBRARY_PATH so the failure
is masked. On some other platforms, this failover doesn't exist so
we get an error when trying to load jdwp and hprof dependent libs.
Some notes on a couple of files.
*
debugInit.c, hprof_init.c*:
Rearranged the init order so that the jvmti pointer gets
initialized before attempting to load the npt shared library.
*linker_md.c, hprof_md.c*
Much of the platform code in hprof and jdwp is duplicated and this
change is no different. Based on the code in hotspot
os_solaris/windows.cpp it parses the boot library path and attempts
to find the library.
*
error_messages.c*
Fixed a bug in the error message code that blindly dereferenced the
npt pointer even if it wasn't set because of an error in loading.
Webrev: http://cr.openjdk.java.net/~bpittore/7200297/webrev.00/
Ran the ute nsk/jdwp nsk/jvmti nsk/hprof tests, the JCK jdwp/jvmti
tests and some command line testing on windows and linux.
thanks,
bill