Have a couple of reviews but still need official reviewer to pass muster.
thanks,
bill
On 11/22/2012 7:51 AM, Dmitry Samersoff wrote:
Bill,
Looks good for me.
-Dmitry
On 2012-11-21 23:31, BILL PITTORE wrote:
Hi Dmitry,
On 11/21/2012 4:56 AM, Dmitry Samersoff wrote:
Bill,
Few nits:
1.
dll_build_name is exactly the same for all locations could we place it
to some common place?
It looked somewhat intentional to have the agents and the hprof code be
self-contained. I looked at using a common file but the makefile changes
and source tree changes seemed a bit much. Hprof code is "unsupported
demo" code and jdwp is supported agent so I went with the current scheme
of having the code be self-contained.
Also file_exists is not necessary and could be
replaced with just ::access(buffer,R_OK) (Windows has it as well)
I'll go with the access suggestion above, less code.
but if you prefer to keep it:
strlen(filename)==0 could be a *filename == 0
2. We don't need else after return in all *_md.c files
e.g. linker_md.c:122
Semantically, you're correct. I think in terms of code readability I
like the 'else' since it makes it clear to the reader that there are two
different cases. C compiler will 'do the right thing'.
Updated the webrev at
http://cr.openjdk.java.net/~bpittore/7200297/webrev.03/
thanks,
bill
Otherwise looks good.
-Dmitry
On 2012-11-20 01:10, BILL PITTORE wrote:
Have gotten reviews from Serguei Spitsyn for the changes, made some
improvements and need an official reviewer to check it out.
http://cr.openjdk.java.net/~bpittore/7200297/webrev.02
thanks,
bill
On 11/14/2012 5: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
