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

[David Holmes]

On 15/11/2018 9:51 pm, Baesken, Matthias 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

Sorry Matthias I misunderstood how the string in STEP was used. The else 
part here is not necessary:

+      if (ExtensiveErrorReports && _verbose) {
+        os::Posix::print_user_info(st);
+      } else {
+        st->print_cr("no extensive user info");
+        st->cr();
+      }

David
-----

> 
> 
> 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
>>>>
>>>