RFR: 8136978 Much nearly duplicated code for vmError support

Sebastian Sickelmann sebastian.sickelmann at gmx.de
Thu Nov 19 10:46:32 UTC 2015


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