8294362: UL: Replace the internal usage of manual buffers with stringStream in LogSelection

Reviewed-by: dholmes, rehn

src/hotspot/share/logging/logOutput.cpp

@@ -32,10 +32,6 @@
 #include "runtime/mutexLocker.hpp"
 #include "runtime/os.hpp"
 
-LogOutput::~LogOutput() {
-  os::free(_config_string);
-}
-
 void LogOutput::describe(outputStream *out) {
   out->print("%s ", name());
   out->print_raw(config_string()); // raw printed because length might exceed O_BUFLEN
@@ -56,34 +52,16 @@ void LogOutput::describe(outputStream *out) {
 }
 
 void LogOutput::set_config_string(const char* string) {
-  os::free(_config_string);
-  _config_string = os::strdup(string, mtLogging);
-  _config_string_buffer_size = strlen(_config_string) + 1;
+  _config_string.reset();
+  _config_string.print_raw(string);
 }
 
 void LogOutput::add_to_config_string(const LogSelection& selection) {
-  if (_config_string_buffer_size < InitialConfigBufferSize) {
-    _config_string_buffer_size = InitialConfigBufferSize;
-    _config_string = REALLOC_C_HEAP_ARRAY(char, _config_string, _config_string_buffer_size, mtLogging);
-  }
-
-  size_t offset = strlen(_config_string);
-  if (offset > 0) {
+  if (_config_string.size() > 0) {
     // Add commas in-between tag and level combinations in the config string
-    _config_string[offset++] = ',';
+    _config_string.print_raw(",");
   }
-
-  for (;;) {
-    int ret = selection.describe(_config_string + offset,
-                                 _config_string_buffer_size - offset);
-    if (ret == -1) {
-      // Double the buffer size and retry
-      _config_string_buffer_size *= 2;
-      _config_string = REALLOC_C_HEAP_ARRAY(char, _config_string, _config_string_buffer_size, mtLogging);
-      continue;
-    }
-    break;
-  };
+  selection.describe_on(&_config_string);
 }
 
 

src/hotspot/share/logging/logOutput.hpp

@@ -29,6 +29,7 @@
 #include "logging/logMessageBuffer.hpp"
 #include "memory/allocation.hpp"
 #include "utilities/globalDefinitions.hpp"
+#include "utilities/ostream.hpp"
 
 class LogDecorations;
 class LogMessageBuffer;
@@ -43,16 +44,13 @@ class LogOutput : public CHeapObj<mtLogging> {
   friend class LogConfiguration;
 
  private:
-  static const size_t InitialConfigBufferSize = 256;
-
   // Track if the output has been reconfigured dynamically during runtime.
   // The status is set each time the configuration of the output is modified,
   // and is reset once after logging initialization is complete.
   // This is only used during logging of the configuration.
   bool _reconfigured;
 
-  char* _config_string;
-  size_t _config_string_buffer_size;
+  stringStream _config_string;
 
   // Adds the log selection to the config description (e.g. "tag1+tag2*=level").
   void add_to_config_string(const LogSelection& selection);
@@ -60,7 +58,7 @@ class LogOutput : public CHeapObj<mtLogging> {
  protected:
   LogDecorators _decorators;
 
-  // Replaces the current config description with the given string.
+  // Replaces the current config description with a copy of the given string.
   void set_config_string(const char* string);
 
   // Update the config string for this output to reflect its current configuration
@@ -80,13 +78,13 @@ class LogOutput : public CHeapObj<mtLogging> {
   }
 
   const char* config_string() const {
-    return _config_string;
+    return _config_string.base();
   }
 
-  LogOutput() : _reconfigured(false), _config_string(NULL), _config_string_buffer_size(0) {
+  LogOutput() : _reconfigured(false), _config_string(){
   }
 
-  virtual ~LogOutput();
+  virtual ~LogOutput() {};
 
   // If the output can be rotated, trigger a forced rotation, otherwise do nothing.
   // Log outputs with rotation capabilities should override this.

src/hotspot/share/logging/logSelection.cpp

@@ -202,36 +202,18 @@ size_t LogSelection::tag_sets_selected() const {
   return _tag_sets_selected;
 }
 
-int LogSelection::describe_tags(char* buf, size_t bufsize) const {
-  int tot_written = 0;
+void LogSelection::describe_tags_on(outputStream* out) const {
   for (size_t i = 0; i < _ntags; i++) {
-    int written = jio_snprintf(buf + tot_written, bufsize - tot_written,
-                               "%s%s", (i == 0 ? "" : "+"), LogTag::name(_tags[i]));
-    if (written == -1) {
-      return written;
-    }
-    tot_written += written;
+    out->print("%s%s", (i == 0 ? "" : "+"), LogTag::name(_tags[i]));
   }
-
   if (_wildcard) {
-    int written = jio_snprintf(buf + tot_written, bufsize - tot_written, "*");
-    if (written == -1) {
-      return written;
-    }
-    tot_written += written;
+    out->print("*");
   }
-  return tot_written;
 }
 
-int LogSelection::describe(char* buf, size_t bufsize) const {
-  int tot_written = describe_tags(buf, bufsize);
-
-  int written = jio_snprintf(buf + tot_written, bufsize - tot_written, "=%s", LogLevel::name(_level));
-  if (written == -1) {
-    return -1;
-  }
-  tot_written += written;
-  return tot_written;
+void LogSelection::describe_on(outputStream* out) const {
+  describe_tags_on(out);
+  out->print("=%s", LogLevel::name(_level));
 }
 
 double LogSelection::similarity(const LogSelection& other) const {
@@ -345,8 +327,7 @@ void LogSelection::suggest_similar_matching(outputStream* out) const {
 
   out->print("Did you mean any of the following?");
   for (size_t i = 0; i < nsuggestions; i++) {
-    char buf[128];
-    suggestions[i].describe_tags(buf, sizeof(buf));
-    out->print(" %s", buf);
+    out->print(" ");
+    suggestions[i].describe_tags_on(out);
   }
 }

src/hotspot/share/logging/logSelection.hpp

@@ -61,8 +61,8 @@ class LogSelection : public StackObj {
   bool selects(const LogTagSet& ts) const;
   bool consists_of(const LogTagType tags[LogTag::MaxTags]) const;
 
-  int describe_tags(char* buf, size_t bufsize) const;
-  int describe(char* buf, size_t bufsize) const;
+  void describe_tags_on(outputStream* out) const;
+  void describe_on(outputStream* out) const;
 
   // List similar selections that matches existing tag sets on the given outputstream
   void suggest_similar_matching(outputStream* out) const;

src/hotspot/share/logging/logSelectionList.cpp

@@ -42,9 +42,9 @@ bool LogSelectionList::verify_selections(outputStream* out) const {
       out->print("No tag set matches selection:");
       valid = false;
 
-      char buf[256];
-      _selections[i].describe_tags(buf, sizeof(buf));
-      out->print(" %s. ", buf);
+      out->print(" ");
+      _selections[i].describe_tags_on(out);
+      out->print(". ");
 
       _selections[i].suggest_similar_matching(out);
       out->cr();

test/hotspot/gtest/logging/test_logSelection.cpp

@@ -27,6 +27,7 @@
 #include "logging/logSelection.hpp"
 #include "logging/logTagSet.hpp"
 #include "utilities/globalDefinitions.hpp"
+#include "utilities/ostream.hpp"
 #include "logTestUtils.inline.hpp"
 #include "unittest.hpp"
 
@@ -41,9 +42,9 @@ void PrintTo(const LogSelection& sel, ::std::ostream* os) {
     *os << "LogSelection::Invalid";
     return;
   }
-  char buf[256];
-  sel.describe(buf, sizeof(buf));
-  *os << buf;
+  stringStream ss;
+  sel.describe_on(&ss);
+  *os << ss.freeze();
 }
 
 TEST(LogSelection, sanity) {
@@ -199,19 +200,19 @@ TEST(LogSelection, consists_of) {
 }
 
 TEST(LogSelection, describe_tags) {
-  char buf[256];
+  stringStream ss;
   LogTagType tags[LogTag::MaxTags] = { PREFIX_LOG_TAG(logging), PREFIX_LOG_TAG(test), PREFIX_LOG_TAG(_NO_TAG) };
   LogSelection selection(tags, true, LogLevel::Off);
-  selection.describe_tags(buf, sizeof(buf));
-  EXPECT_STREQ("logging+test*", buf);
+  selection.describe_tags_on(&ss);
+  EXPECT_STREQ("logging+test*", ss.freeze());
 }
 
 TEST(LogSelection, describe) {
-  char buf[256];
+  stringStream ss;
   LogTagType tags[LogTag::MaxTags] = { PREFIX_LOG_TAG(logging), PREFIX_LOG_TAG(test), PREFIX_LOG_TAG(_NO_TAG) };
   LogSelection selection(tags, true, LogLevel::Off);
-  selection.describe(buf, sizeof(buf));
-  EXPECT_STREQ("logging+test*=off", buf);
+  selection.describe_on(&ss);
+  EXPECT_STREQ("logging+test*=off", ss.freeze());
 }
 
 #endif