RFR: 8146115 - Improve docker container detection and resource configuration usage

[Kim Barrett]

> On Oct 24, 2017, at 10:11 AM, Bob Vandette <bob.vandette at oracle.com> wrote:
> 
> 
>> On Oct 23, 2017, at 12:52 AM, Kim Barrett <kim.barrett at oracle.com> wrote:
>> 
>>> On Sep 27, 2017, at 9:20 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>>> 62     void set_subsystem_path(char *cgroup_path) {
>>>>> 
>>>>> If this takes a "const char*" will it save you from casting string literals to "char*" elsewhere?
>>>> I tried several different ways of declaring the container accessor functions and
>>>> always ended up with warnings due to scanf not being able to validate arguments
>>>> since the format string didn’t end up being a string literal.  I originally was using templates
>>>> and then ended up with the macros.  I tried several different casts but could resolve the problem.
>>> 
>>> Sounds like something Kim Barrett should take a look at :)
>> 
>> Fortunately, I just happened by.
>> 
>> The warnings are because we compile with -Wformat=2, which enables
>> -Wformat-nonliteral (among other things).
>> 
>> Use PRAGMA_FORMAT_NONLITERAL_IGNORED, e.g.
>> 
>> PRAGMA_DIAG_PUSH
>> PRAGMA_FORMAT_NONLITERAL_IGNORED
>> <function definition>
>> PRAGMA_DIAG_POP
>> 
>> That will silence warnings about sscanf (or anything else!) with a
>> non-literal format string within that <function definition>.
> 
> Thanks but I ended up taking a different approach that resulted in more compact code.
> 
> http://cr.openjdk.java.net/~bobv/8146115/webrev.02

Not a review, just a few more comments in passing.

------------------------------------------------------------------------------ 
src/hotspot/os/linux/osContainer_linux.cpp 
 150             log_debug(os, container)("Type %s not found in file %s\n",    \
 151                                      scan_fmt , buf);                     \

uses buf as path, but buf has been clobbered to contain contents from
file.

Similarly for
 155         log_debug(os, container)("Empty file %s\n", buf);                 \

------------------------------------------------------------------------------
src/hotspot/os/linux/osContainer_linux.cpp 
 158       log_debug(os, container)("file not found %s\n", buf);               \

There are many reasons why fopen might fail, and merging them all into
a "file not found" message could be quite confusing.  It would be much
better to report the error from errno.

------------------------------------------------------------------------------ 
src/hotspot/os/linux/osContainer_linux.cpp 

Something like the following (where the obvious helpers are made up to
keep this short) would eliminate the macrology.

PRAGMA_DIAG_PUSH  
PRAGMA_FORMAT_NONLITERAL_IGNORED
template<typename T>
int get_subsystem_file_contents_value(CgroupSubsystem* c,
                                  const char* filename,
                                  T* returnval,
                                  const char* scan_fmt,
				  const char* description) {
  const char* line = get_subsystem_file_line(c, filename);
  if (line != NULL) {
    if (sscanf(line, scan_fmt, returnval) == 1) {
      return 0;
    } else {
      report_subsystem_file_contents_parse_error(description, c, filename);
    }
  }
  return OSCONTAINER_ERROR;
}
PRAGMA_DIAG_POP

int subsystem_file_contents_int(CgroupSubsystem* c,
                                const char* filename,
                                int* returnval) {
  return get_subsystem_file_contents_value(c, filename, returnval, "%d", "int");
}

------------------------------------------------------------------------------