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

[Thomas Stüfe]

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