RFR [XS]: 8224958: add os::dll_load calls to event log

[Thomas Stüfe]

On Wed, Jun 5, 2019 at 11:06 AM Baesken, Matthias <matthias.baesken at sap.com>
wrote:

> Hi Thomas , I removed  the error_report _ NULL check
>
>
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8224958.2/
>
>
>
>    - You moved lasterror() up to get a "fresh" error value before calling
>    into event logging right? I needed a while to understand that. Okay.
>
>
>
> Yes this was my intention .
>
>
>
>    - It would be better to have a function which only does the text
>    translation, using an error code we give it as parameter.
>    - Then we could have given it the errcode we already have. Would be
>    good for a future improvement.
>
>
>
> There are not so many  os::lasterror   usages currently .  So I am not
> sure if it is really needed ,   let’s  do it in a separate CR  any way …
>
>
>
>
>

Yeah sure. Maybe we can remove it altogether.

Patch looks good to me.

Cheers, Thomas




>
>
> Best regards, Matthias
>
>
>
>
>
> *From:* Thomas Stüfe <thomas.stuefe at gmail.com>
> *Sent:* Mittwoch, 5. Juni 2019 09:31
> *To:* Baesken, Matthias <matthias.baesken at sap.com>
> *Cc:* hotspot-dev at openjdk.java.net
> *Subject:* Re: RFR [XS]: 8224958: add os::dll_load calls to event log
>
>
>
> Hi Matthias,
>
> looks good. Minor nit:
>
>
> +  if (error_report == NULL) {
> +    error_report = "dlerror returned no error description";
> +  }
> +  if (error_report && ebuf && ebuflen > 0) {
>
> Since you ensure error_report is always set - with a default text, if
> needed - you do not need to check it for NULL afterwards.
>
> If you only fix this, I do not need a new webrev.
>
>
> ---
>
> Side note:
>
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8224958.1/src/hotspot/os/windows/os_windows.cpp.udiff.html
>
> You moved lasterror() up to get a "fresh" error value before calling into
> event logging right? I needed a while to understand that. Okay.
>
> This shows that lasterror() is actually not well designed. lasterror()
> does two things: 1) get the error code with GetLastError and 2) translate
> that numerical code to a text.
>
> It would be better to have a function which only does the text
> translation, using an error code we give it as parameter. Then we could
> have given it the errcode we already have. Would be good for a future
> improvement.
>
> Cheers, Thomas
>
>
>
>
>