--- a/hotspot/src/share/vm/logging/log.cpp Wed Aug 24 19:21:20 2016 +0300
+++ b/hotspot/src/share/vm/logging/log.cpp Mon Aug 29 14:11:22 2016 +0200
@@ -1161,7 +1161,7 @@
// 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,
--- a/hotspot/src/share/vm/logging/logConfiguration.cpp Wed Aug 24 19:21:20 2016 +0300
+++ b/hotspot/src/share/vm/logging/logConfiguration.cpp Mon Aug 29 14:11:22 2016 +0200
@@ -44,6 +44,9 @@
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 @@
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 @@
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 @@
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();
--- a/hotspot/src/share/vm/logging/logConfiguration.hpp Wed Aug 24 19:21:20 2016 +0300
+++ b/hotspot/src/share/vm/logging/logConfiguration.hpp Mon Aug 29 14:11:22 2016 +0200
@@ -59,7 +59,7 @@
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);
--- a/hotspot/src/share/vm/logging/logFileOutput.cpp Wed Aug 24 19:21:20 2016 +0300
+++ b/hotspot/src/share/vm/logging/logFileOutput.cpp Mon Aug 29 14:11:22 2016 +0200
@@ -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 @@
_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) {
--- a/hotspot/src/share/vm/logging/logFileOutput.hpp Wed Aug 24 19:21:20 2016 +0300
+++ b/hotspot/src/share/vm/logging/logFileOutput.hpp Mon Aug 29 14:11:22 2016 +0200
@@ -91,6 +91,7 @@
return _name;
}
+ static const char* Prefix;
static void set_file_name_parameters(jlong start_time);
};
--- a/hotspot/test/native/logging/test_logConfiguration.cpp Wed Aug 24 19:21:20 2016 +0300
+++ b/hotspot/test/native/logging/test_logConfiguration.cpp Mon Aug 29 14:11:22 2016 +0200
@@ -308,3 +308,30 @@
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);
+}
--- a/hotspot/test/native/logging/test_logFileOutput.cpp Wed Aug 24 19:21:20 2016 +0300
+++ b/hotspot/test/native/logging/test_logFileOutput.cpp Mon Aug 29 14:11:22 2016 +0200
@@ -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) {