RFR (S): JDK-8056242: Add function to return structured information about loaded libraries.

[Fredrik Arvidsson]

Hi

Heres the latest version of my patch. I have learned so much these last 
couple of days. I'm very grateful...

Webrev: http://cr.openjdk.java.net/~farvidsson/8056242/webrev.05/index.html

Dmitry: I don't really know how to test the BSD changes. Do you have an 
environment for that?

Cheers

/F

On 2014-09-01 15:12, Staffan Larsen wrote:
> Thanks for doing these changes. Here are comments on the new version:
>
> os_windows.cpp
> - Why not rename enumerate_modules() to get_loaded_modules_info() and update all caller to use the new unified method?
> - At the same time, can we get rid of the pid parameter to enumerate_modules()? It looks like all callers set it the same value.
> - nit: looks like the <= and > used to be aligned in these if-statements:
> 1380    if (base_addr     <= pmod->addr &&
> 1381        top_address > pmod->addr) {
> 1436    if (base_addr     <= (address)_locate_jvm_dll &&
> 1437        top_address > (address)_locate_jvm_dll) {
>
> os_solaris.cpp
> - I think print_dll_info() should be re-implemented in terms of get_loaded_modules_info() so that there is only one implementation of this logic.
> - get_loaded_modules_info() references os::print_dll_info() which looks like a mistake?
>
> os_linux.cpp
> - get_loaded_modules_info() could use some blank lines to split it up into logical chunks of code.
> - I think “r” is enough for fopen() - ie drop the “t”
> - Can the fgets+sscanf be replaced by a call to fscanf? That would avoid the ‘line’ variable and any possible problems of lines not fitting into the space allocated.
>
> os_bsd.cpp
> - Maybe add a comment about module_top_addr being 0?
>
> Thanks,
> /Staffan
>
>
> On 1 sep 2014, at 14:05, Fredrik Arvidsson <fredrik.arvidsson at oracle.com> wrote:
>
>> Hi
>>
>> Here is an updated version of the patch. Thanks for the comments.
>>
>> Webrev: http://cr.openjdk.java.net/~farvidsson/8056242/webrev.01/index.html
>>
>> /Fredrik
>>
>> On 2014-08-28 16:21, Staffan Larsen wrote:
>>> Hi Fredrik,
>>>
>>> A couple of comments:
>>> - I would prefer if the new callback was unified with the one that exists on Windows so that we have only on callback-based API for listing dynamic libraries.
>>> - If you do that, then would you also clean up enumerate_modules() on windows to get rid of the non-NT support?
>>> - Smaller: I think “address” is a better type to use instead of “u8”.
>>>
>>> Thanks
>>> /Staffan
>>>
>>>
>>> On 28 aug 2014, at 15:54, Fredrik Arvidsson <fredrik.arvidsson at oracle.com> wrote:
>>>
>>>> Hi
>>>>
>>>> Please help me review this small enhancement.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~farvidsson/8056242/webrev.00/index.html
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8056242
>>>>
>>>> Cheers
>>>> /F