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

Thomas Stüfe thomas.stuefe at gmail.com
Fri Sep 21 16:59:10 UTC 2018

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

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

> Thanks, Thomas
> On Fri, Sep 21, 2018 at 11:13 AM, Baesken, Matthias
> <matthias.baesken at sap.com> wrote:
>> Hello , please review this small patch .
>> It adds more information  about  pre-loading configs  of libs on Linux  (often used for  monitoring tools).
>> Currently the hs_err file contains  already the LD_PRELOAD environment variable.
>> However for system wide configuration, the /etc/ld.so.preload file is important as well on Linux. So display it as well (in case it is present).
>> See the  Environment / LD_PRELOAD   and FILES   sections of :
>> http://man7.org/linux/man-pages/man8/ld.so.8.html
>> for more information .
>> Bug/webrev :
>> https://bugs.openjdk.java.net/browse/JDK-8210964
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8210964.0/
>> Thanks, Matthias

More information about the hotspot-dev mailing list