RFR : 8211326 : add OS user related information to hs_err file

[David Holmes]

On 15/11/2018 10:33 pm, Thomas Stüfe wrote:
> I'm fine with this, Matthias.
> 
> BTW, I looked and the STEP header line is only printed when secondary
> exceptions happen inside the step, not always as David assumed (?). So

Yes my bad - sorry.

> your first form
> 
> STEP("blabla")
>    if (ExtensiveErrorReports && _verbose) {
>       blub
>    }
> 
> would have been fine too I think. But I do not want to send you
> through another review iteration, so I am fine with this version.

Deleting the else doesn't need another round of review.

Thanks,
David


> Thanks for incorporating my remarks.
> 
> Cheers,Thomas
> 
> 
> On Thu, Nov 15, 2018 at 12:51 PM Baesken, Matthias
> <matthias.baesken at sap.com> wrote:
>>
>> Hi Thomas,
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.3/
>>
>>
>> - got  rid of buf+buflen arguments of print_user_info
>> - removed "Removing"
>> - adjusted the output a bit   in  os::Posix::print_user_info()
>> - vmError.cpp  added a short  output line  with info in case   ( ExtensiveErrorReports && _verbose  )  is false
>>
>>
>> Best regards, Matthias
>>
>>
>>> -----Original Message-----
>>> From: Thomas Stüfe <thomas.stuefe at gmail.com>
>>> Sent: Mittwoch, 14. November 2018 18:43
>>> To: Baesken, Matthias <matthias.baesken at sap.com>
>>> Cc: David Holmes <david.holmes at oracle.com>; Langer, Christoph
>>> <christoph.langer at sap.com>; Volker Simonis <volker.simonis at gmail.com>;
>>> HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
>>> Subject: Re: RFR : 8211326 : add OS user related information to hs_err file
>>>
>>> Hi Matthias,
>>>
>>> none of my notes from my earlier mail are addressed? I guess my mail
>>> got lost in all that discussion. Here you go:
>>>
>>> +  static void print_user_info(outputStream* st, char* buf, size_t buflen);
>>>
>>> You can get rid of buf+buflen arguments, not needed anymore
>>> ---
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.2/src/hotspot/os/
>>> posix/os_posix.cpp.udiff.html
>>>
>>> In os::Posix::print_user_info(), for brevity, I would print it all one
>>> line like this:  "uid: 4711, euid: 0, euid: 4711, egid: 4711, umask:
>>> rwxr--r--"
>>>
>>> ---
>>>
>>> The "removing" text does not make sense and can be removed.
>>>
>>> Best Regards, Thomas
>>>
>>>
>>> On Wed, Nov 14, 2018 at 6:16 PM Baesken, Matthias
>>> <matthias.baesken at sap.com> wrote:
>>>>
>>>> Hi Thomas , so are you fine with the latest revision
>>>>
>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.1/
>>>>
>>>> of the patch ,  can I add you as reviewer ?
>>>>
>>>> David - is the info added by Thomas sufficient for you?
>>>>
>>>> Any other reviewers ?
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Thomas Stüfe <thomas.stuefe at gmail.com>
>>>>> Sent: Samstag, 10. November 2018 14:04
>>>>> To: David Holmes <david.holmes at oracle.com>
>>>>> Cc: Baesken, Matthias <matthias.baesken at sap.com>; Langer, Christoph
>>>>> <christoph.langer at sap.com>; Volker Simonis
>>> <volker.simonis at gmail.com>;
>>>>> HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
>>>>> Subject: Re: RFR : 8211326 : add OS user related information to hs_err file
>>>>>
>>>>> Hi David,
>>>>>
>>>>> On Sat, Nov 10, 2018 at 12:59 PM David Holmes
>>> <david.holmes at oracle.com>
>>>>> wrote:
>>>>>>
>>>>>> Hi Matthias,
>>>>>>
>>>>>> On 9/11/2018 3:13 AM, Baesken, Matthias wrote:
>>>>>>> Hello  , after the flag   "-XX:+-ExtensiveErrorReports"   make it into
>>> jdk/jdk
>>>>> , I created another webrev :
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.1/
>>>>>>>
>>>>>>>
>>>>>>> The user info output is now guarded  by ExtensiveErrorReports .
>>>>>>
>>>>>> +   STEP("printing user info")
>>>>>> +      if (ExtensiveErrorReports) {
>>>>>> +        if (_verbose) {
>>>>>> +          os::Posix::print_user_info(st, buf, sizeof(buf));
>>>>>> +        }
>>>>>> +      }
>>>>>>
>>>>>> I don't understand why we explicitly need _verbose if we've asked for
>>>>>> ExtensiveErrorreports?
>>>>>
>>>>> That flag has a different purpose: _verbose distinguishes the two
>>>>> calls to VMError::report(): the first one to print a small report to
>>>>> stderr, the second print a large report to the hs-err file. We only
>>>>> want to print to the hs-err file, so _verbose is still needed.
>>>>>
>>>>>>
>>>>>> Also, ideally the STEP would be inside the guard as otherwise we will
>>>>>> just print the step description followed by nothing. If the macro
>>>>>> expansion makes that impossible then we should have an else clause
>>> that
>>>>>> prints something like:
>>>>>>
>>>>>>    - disabled (use -XX:+ExtensiveErrorReports to see these details)
>>>>>>
>>>>>
>>>>> We may even go a step further (but in a separate patch) and make
>>>>> "ExtensiveErrorReports" a property of the STEP, to be given as
>>>>> parameter of the STEP macro.
>>>>>
>>>>> Thanks, Thomas
>>>>>
>>>>