RFR: 8136978 Much nearly duplicated code for vmError support

Coleen Phillimore coleen.phillimore at oracle.com
Thu Nov 19 14:50:08 UTC 2015


JPRT is our build and test system that runs all the platforms we 
support.  It runs a subset of our tests.  It's how we integrate code so 
that no code that doesn't build and pass tests can get integrated.

I think you mean 03.

http://cr.openjdk.java.net/~sebastian/8136978/webrev.03/index.html

I verified all the code.  It looks great!  I'm happy to sponsor, pending 
another code review.

Thanks,
Coleen



On 11/19/15 5:46 AM, Sebastian Sickelmann wrote:
> Hi Coleen,
>
> thanks for finding those errors. I am sorry i missed those. It shows
> that it is really helpful two test/compile on multiple platforms.
> What is JPRT?
>
> Please find the new webrev here:
>
> http://cr.openjdk.java.net/~sebastian/8136978/webrev.02
>
> thanks,
> Sebastian
>
> On 11/19/2015 05:59 AM, Coleen Phillimore wrote:
>> Hi Sebastian,
>>
>> I tested your change through JPRT and there are a few changes I needed
>> to make to get it to pass.
>>
>> One is that I changed os::message_box to return bool since that's how
>> it's used and the use in vmError.cpp made the Windows compiler complain.
>>
>> Others (hope you can read tell the diffs).   There was thr_sigsetmask
>> on solaris and a couple places os::format_debug_message() you had 'p'
>> rather than 'buf' and one place had 'buflen' rather than 80.  Those
>> places os::start_debugging(), it would be better to have sizeof(buf)
>> rather than 80.
>>
>> Otherwise, Reviewed.  If you do create a changeset, I will sponsor
>> after another reviewer sees it.
>>
>> thanks,
>> Coleen
>>
>>
>> < --- old/src/os/solaris/vm/os_solaris.cpp    2015-11-18
>> 19:05:31.209186804 +0100
>> < +++ new/src/os/solaris/vm/os_solaris.cpp    2015-11-18
>> 19:05:31.129186803 +0100
>> ---
>>> diff --git a/src/os/solaris/vm/os_solaris.cpp
>> b/src/os/solaris/vm/os_solaris.cpp
>>> --- a/src/os/solaris/vm/os_solaris.cpp
>>> +++ b/src/os/solaris/vm/os_solaris.cpp
>>> @@ -3611,7 +3611,7 @@
>>>   void os::print_statistics() {
>>>   }
>>>
>>> -int os::message_box(const char* title, const char* message) {
>>> +bool os::message_box(const char* title, const char* message) {
>>>     int i;
>>>     fdStream err(defaultStream::error_fd());
>>>     for (i = 0; i < 78; i++) err.print_raw("=");
>> 195c239
>> < +  return sigsetmask(SIG_UNBLOCK, set, NULL);
>> ---
>>> +  return thr_sigsetmask(SIG_UNBLOCK, set, NULL);
>> 199c243
>> < +  jio_snprintf(p, buflen,
>> ---
>>> +  jio_snprintf(buf, buflen,
>> 210c254
>> < +  jio_snprintf(buf, buflen, "dbx - %d", os::current_process_id());
>> ---
>>> +  jio_snprintf(buf, 80, "dbx - %d", os::current_process_id());
>> < --- old/src/os/windows/vm/os_windows.cpp    2015-11-18
>> 19:05:31.977186818 +0100
>> < +++ new/src/os/windows/vm/os_windows.cpp    2015-11-18
>> 19:05:31.829186815 +0100
>> ---
>>> diff --git a/src/os/windows/vm/os_windows.cpp
>> b/src/os/windows/vm/os_windows.cpp
>>> --- a/src/os/windows/vm/os_windows.cpp
>>> +++ b/src/os/windows/vm/os_windows.cpp
>>> @@ -4005,7 +4005,7 @@
>>>   }
>>>
>>>
>>> -int os::message_box(const char* title, const char* message) {
>>> +bool os::message_box(const char* title, const char* message) {
>>>     int result = MessageBox(NULL, message, title,
>>>                             MB_YESNO | MB_ICONERROR | MB_SYSTEMMODAL
>> | MB_DEFAULT_DESKTOP_ONLY);
>>>     return result == IDYES;
>> 230a286
>>> -
>> 232c288
>> < +  jio_snprintf(p, buflen,
>> ---
>>> +  jio_snprintf(buf, buflen,
>>
>>
>>
>>
>> On 11/18/15 1:15 PM, Sebastian Sickelmann wrote:
>>> Hi,
>>>
>>> Coleen found some places where I missed some refactoring due to the
>>> rebase of the initial patch.
>>> Thanks for reporting it to me.
>>>
>>> I hope this here is fine now on every platform:
>>>
>>> http://cr.openjdk.java.net/~sebastian/8136978/webrev.02/
>>>
>>> Sorry for the inconvenience.
>>>
>>> -- 
>>> Sebastian
>>



More information about the hotspot-runtime-dev mailing list