RFR [XS] 8210964: add more ld preloading info to hs_error file on Linux

[Baesken, Matthias]

Hi Thomas,  thanks for your input !

I followed your suggestions :
- renamed  the function to os::Linux::print_ld_preload_file
- output  the header only in case the file is opened
- changed (shortened) the header 

> > Also, I personally would not expose it via os_linux.hpp.

However I kept it there,  to keep it in line with the other info-functions.

New webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8210964.1/


Best regards, Matthias



> -----Original Message-----
> From: Thomas Stüfe <thomas.stuefe at gmail.com>
> Sent: Freitag, 21. September 2018 18:59
> To: Baesken, Matthias <matthias.baesken at sap.com>
> Cc: hotspot-dev at openjdk.java.net
> Subject: Re: RFR [XS] 8210964: add more ld preloading info to hs_error file on
> Linux
> 
> On Fri, Sep 21, 2018 at 6:47 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
> > Hi Matthias,
> >
> > in the posted patch the new function is never called, or am I mistaken?
> >
> 
> Scratch that, I was being blind.
> 
> What I do not like is the very generic name of the function. It does
> one very specific thing, so could we name it accordingly, e.g.
> "os::Linux::print_system_config_info" ->
> "os::Linux::print_ld_preload_file"?
> 
> Also, I personally would not expose it via os_linux.hpp. I would just
> put it as a file scope static function into os_linux.cpp. I see you
> follow the example of others here (os::Linux::print_proc_sys_info etc)
> but there too I feel this is a bad pattern and those functions do not
> belong into an external header.
> 
> 2140 st->print_cr("/etc/ld.so.preload (system-wide LD_PRELOAD config
> file):");
> 
> I would shorten that to just "ld.so.preload:" or "/etc/ld.so.preload".
> Everyone looking at that section knows what that file does. I also
> would omit that header completely if the file cannot be opened.
> 
> 2141   bool file_present = _print_ascii_file("/etc/ld.so.preload", st);
> 2142   if (!file_present) { st->print("file not present"); }
> 2143   st->cr();
> 
> You do not know that the file is not preset, all you know is that the
> open() call in _print_ascii_file() failed. Since existence of this
> file is optional, and not typical, I would just omit messages if it
> cannot be found.
> 
> Best Regards, Thomas
>