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

Kim Barrett kim.barrett at oracle.com
Wed Oct 25 06:57:14 UTC 2017

> 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).
>> <function definition>
>> 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.

 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

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

 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.


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

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

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


More information about the hotspot-dev mailing list