Merge

hotspot/src/share/vm/logging/log.cpp

@@ -1161,7 +1161,7 @@ void Test_invalid_log_file() {
 
   // Attempt to log to a directory (existing log not a regular file)
   create_directory(target_name);
-  LogFileOutput bad_file("tmplogdir");
+  LogFileOutput bad_file("file=tmplogdir");
   assert(bad_file.initialize("", &ss) == false, "file was initialized "
          "when there was an existing directory with the same name");
   assert(strstr(ss.as_string(), "tmplogdir is not a regular file") != NULL,

hotspot/src/share/vm/logging/logConfiguration.cpp

@@ -44,6 +44,9 @@ size_t      LogConfiguration::_n_outputs = 0;
 LogConfiguration::UpdateListenerFunction* LogConfiguration::_listener_callbacks = NULL;
 size_t      LogConfiguration::_n_listener_callbacks = 0;
 
+// LogFileOutput is the default type of output, its type prefix should be used if no type was specified
+static const char* implicit_output_prefix = LogFileOutput::Prefix;
+
 // Stack object to take the lock for configuring the logging.
 // Should only be held during the critical parts of the configuration
 // (when calling configure_output or reading/modifying the outputs array).
@@ -107,6 +110,55 @@ void LogConfiguration::finalize() {
   FREE_C_HEAP_ARRAY(LogOutput*, _outputs);
 }
 
+// Normalizes the given LogOutput name to type=name form.
+// For example, foo, "foo", file="foo", will all be normalized to file=foo (no quotes, prefixed).
+static bool normalize_output_name(const char* full_name, char* buffer, size_t len, outputStream* errstream) {
+  const char* start_quote = strchr(full_name, '"');
+  const char* equals = strchr(full_name, '=');
+  const bool quoted = start_quote != NULL;
+  const bool is_stdout_or_stderr = (strcmp(full_name, "stdout") == 0 || strcmp(full_name, "stderr") == 0);
+
+  // ignore equals sign within quotes
+  if (quoted && equals > start_quote) {
+    equals = NULL;
+  }
+
+  const char* prefix = "";
+  size_t prefix_len = 0;
+  const char* name = full_name;
+  if (equals != NULL) {
+    // split on equals sign
+    name = equals + 1;
+    prefix = full_name;
+    prefix_len = equals - full_name + 1;
+  } else if (!is_stdout_or_stderr) {
+    prefix = implicit_output_prefix;
+    prefix_len = strlen(prefix);
+  }
+  size_t name_len = strlen(name);
+
+  if (quoted) {
+    const char* end_quote = strchr(start_quote + 1, '"');
+    if (end_quote == NULL) {
+      errstream->print_cr("Output name has opening quote but is missing a terminating quote.");
+      return false;
+    }
+    if (start_quote != name || end_quote[1] != '\0') {
+      errstream->print_cr("Output name can not be partially quoted."
+                          " Either surround the whole name with quotation marks,"
+                          " or do not use quotation marks at all.");
+      return false;
+    }
+    // strip start and end quote
+    name++;
+    name_len -= 2;
+  }
+
+  int ret = jio_snprintf(buffer, len, "%.*s%.*s", prefix_len, prefix, name_len, name);
+  assert(ret > 0, "buffer issue");
+  return true;
+}
+
 size_t LogConfiguration::find_output(const char* name) {
   for (size_t i = 0; i < _n_outputs; i++) {
     if (strcmp(_outputs[i]->name(), name) == 0) {
@@ -116,39 +168,14 @@ size_t LogConfiguration::find_output(const char* name) {
   return SIZE_MAX;
 }
 
-LogOutput* LogConfiguration::new_output(char* name, const char* options, outputStream* errstream) {
-  const char* type;
-  char* equals_pos = strchr(name, '=');
-  if (equals_pos == NULL) {
-    type = "file";
-  } else {
-    *equals_pos = '\0';
-    type = name;
-    name = equals_pos + 1;
-  }
-
-  // Check if name is quoted, and if so, strip the quotes
-  char* quote = strchr(name, '"');
-  if (quote != NULL) {
-    char* end_quote = strchr(name + 1, '"');
-    if (end_quote == NULL) {
-      errstream->print_cr("Output name has opening quote but is missing a terminating quote.");
-      return NULL;
-    } else if (quote != name || end_quote[1] != '\0') {
-      errstream->print_cr("Output name can not be partially quoted."
-                          " Either surround the whole name with quotation marks,"
-                          " or do not use quotation marks at all.");
-      return NULL;
-    }
-    name++;
-    *end_quote = '\0';
-  }
-
+LogOutput* LogConfiguration::new_output(const char* name,
+                                        const char* options,
+                                        outputStream* errstream) {
   LogOutput* output;
-  if (strcmp(type, "file") == 0) {
+  if (strncmp(name, LogFileOutput::Prefix, strlen(LogFileOutput::Prefix)) == 0) {
     output = new LogFileOutput(name);
   } else {
-    errstream->print_cr("Unsupported log output type.");
+    errstream->print_cr("Unsupported log output type: %s", name);
     return NULL;
   }
 
@@ -374,25 +401,35 @@ bool LogConfiguration::parse_log_arguments(const char* outputstr,
 
   ConfigurationLock cl;
   size_t idx;
-  if (outputstr[0] == '#') {
-    int ret = sscanf(outputstr+1, SIZE_FORMAT, &idx);
+  if (outputstr[0] == '#') { // Output specified using index
+    int ret = sscanf(outputstr + 1, SIZE_FORMAT, &idx);
     if (ret != 1 || idx >= _n_outputs) {
       errstream->print_cr("Invalid output index '%s'", outputstr);
       return false;
     }
-  } else {
-    idx = find_output(outputstr);
+  } else { // Output specified using name
+    // Normalize the name, stripping quotes and ensures it includes type prefix
+    size_t len = strlen(outputstr) + strlen(implicit_output_prefix) + 1;
+    char* normalized = NEW_C_HEAP_ARRAY(char, len, mtLogging);
+    if (!normalize_output_name(outputstr, normalized, len, errstream)) {
+      return false;
+    }
+
+    idx = find_output(normalized);
     if (idx == SIZE_MAX) {
-      char* tmp = os::strdup_check_oom(outputstr, mtLogging);
-      LogOutput* output = new_output(tmp, output_options, errstream);
-      os::free(tmp);
-      if (output == NULL) {
-        return false;
+      // Attempt to create and add the output
+      LogOutput* output = new_output(normalized, output_options, errstream);
+      if (output != NULL) {
+        idx = add_output(output);
       }
-      idx = add_output(output);
     } else if (output_options != NULL && strlen(output_options) > 0) {
       errstream->print_cr("Output options for existing outputs are ignored.");
     }
+
+    FREE_C_HEAP_ARRAY(char, normalized);
+    if (idx == SIZE_MAX) {
+      return false;
+    }
   }
   configure_output(idx, expr, decorators);
   notify_update_listeners();

hotspot/src/share/vm/logging/logConfiguration.hpp

@@ -59,7 +59,7 @@ class LogConfiguration : public AllStatic {
   static size_t                     _n_listener_callbacks;
 
   // Create a new output. Returns NULL if failed.
-  static LogOutput* new_output(char* name, const char* options, outputStream* errstream);
+  static LogOutput* new_output(const char* name, const char* options, outputStream* errstream);
 
   // Add an output to the list of configured outputs. Returns the assigned index.
   static size_t add_output(LogOutput* out);

hotspot/src/share/vm/logging/logFileOutput.cpp

@@ -31,6 +31,7 @@
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/defaultStream.hpp"
 
+const char* LogFileOutput::Prefix = "file=";
 const char* LogFileOutput::FileOpenMode = "a";
 const char* LogFileOutput::PidFilenamePlaceholder = "%p";
 const char* LogFileOutput::TimestampFilenamePlaceholder = "%t";
@@ -45,7 +46,8 @@ LogFileOutput::LogFileOutput(const char* name)
       _file_name(NULL), _archive_name(NULL), _archive_name_len(0),
       _rotate_size(DefaultFileSize), _file_count(DefaultFileCount),
       _current_size(0), _current_file(0), _rotation_semaphore(1) {
-  _file_name = make_file_name(name, _pid_str, _vm_start_time_str);
+  assert(strstr(name, Prefix) == name, "invalid output name '%s': missing prefix: %s", name, Prefix);
+  _file_name = make_file_name(name + strlen(Prefix), _pid_str, _vm_start_time_str);
 }
 
 void LogFileOutput::set_file_name_parameters(jlong vm_start_time) {

hotspot/src/share/vm/logging/logFileOutput.hpp

@@ -91,6 +91,7 @@ class LogFileOutput : public LogFileStreamOutput {
     return _name;
   }
 
+  static const char* Prefix;
   static void set_file_name_parameters(jlong start_time);
 };
 

hotspot/test/native/logging/test_logConfiguration.cpp

@@ -308,3 +308,30 @@ TEST_F(LogConfigurationTest, parse_invalid_tagset) {
   EXPECT_TRUE(string_contains_substring(msg, "No tag set matches selection(s):"));
   EXPECT_TRUE(string_contains_substring(msg, invalid_tagset));
 }
+
+TEST_F(LogConfigurationTest, output_name_normalization) {
+  const char* patterns[] = { "%s", "file=%s", "\"%s\"", "file=\"%s\"" };
+  char buf[1 * K];
+  for (size_t i = 0; i < ARRAY_SIZE(patterns); i++) {
+    int ret = jio_snprintf(buf, sizeof(buf), patterns[i], TestLogFileName);
+    ASSERT_NE(-1, ret);
+    set_log_config(buf, "logging=trace");
+    EXPECT_TRUE(is_described("#2: "));
+    EXPECT_TRUE(is_described(TestLogFileName));
+    EXPECT_FALSE(is_described("#3: "))
+        << "duplicate file output due to incorrect normalization for pattern: " << patterns[i];
+  }
+
+  // Make sure prefixes are ignored when used within quotes
+  // (this should create a log with "file=" in its filename)
+  int ret = jio_snprintf(buf, sizeof(buf), "\"file=%s\"", TestLogFileName);
+  ASSERT_NE(-1, ret);
+  set_log_config(buf, "logging=trace");
+  EXPECT_TRUE(is_described("#3: ")) << "prefix within quotes not ignored as it should be";
+  set_log_config(buf, "all=off");
+
+  // Remove the extra log file created
+  ret = jio_snprintf(buf, sizeof(buf), "file=%s", TestLogFileName);
+  ASSERT_NE(-1, ret);
+  delete_file(buf);
+}

hotspot/test/native/logging/test_logFileOutput.cpp

@@ -29,7 +29,7 @@
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/ostream.hpp"
 
-static const char* name = "testlog.pid%p.%t.log";
+static const char* name = "file=testlog.pid%p.%t.log";
 
 // Test parsing a bunch of valid file output options
 TEST(LogFileOutput, parse_valid) {