8146793: logStream::write re-formats string
authorkbarrett
Fri, 29 Jan 2016 20:57:09 -0500
changeset 35897 beba2418d973
parent 35896 44c25fd8dbb3
child 35898 ddc274f0052f
8146793: logStream::write re-formats string Summary: Eliminate re-format, add warning attribute, fix size check, fix va_list usage. Reviewed-by: mlarsson, rprotacio, jrose
hotspot/src/share/vm/logging/log.hpp
hotspot/src/share/vm/utilities/ostream.cpp
hotspot/src/share/vm/utilities/ostream.hpp
--- a/hotspot/src/share/vm/logging/log.hpp	Fri Jan 29 16:25:10 2016 -0800
+++ b/hotspot/src/share/vm/logging/log.hpp	Fri Jan 29 20:57:09 2016 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -120,15 +120,17 @@
   ATTRIBUTE_PRINTF(1, 0)
   static void vwrite(const char* fmt, va_list args) {
     char buf[LogBufferSize];
+    va_list saved_args;         // For re-format on buf overflow.
+    va_copy(saved_args, args);
     size_t prefix_len = LogPrefix<T0, T1, T2, T3, T4>::prefix(buf, sizeof(buf));
     // Check that string fits in buffer; resize buffer if necessary
     int ret = os::log_vsnprintf(buf + prefix_len, sizeof(buf) - prefix_len, fmt, args);
     assert(ret >= 0, "Log message buffer issue");
-    if ((size_t)ret > sizeof(buf)) {
+    if ((size_t)ret >= sizeof(buf)) {
       size_t newbuf_len = prefix_len + ret + 1;
       char* newbuf = NEW_C_HEAP_ARRAY(char, newbuf_len, mtLogging);
       prefix_len = LogPrefix<T0, T1, T2, T3, T4>::prefix(newbuf, newbuf_len);
-      ret = os::log_vsnprintf(newbuf + prefix_len, newbuf_len - prefix_len, fmt, args);
+      ret = os::log_vsnprintf(newbuf + prefix_len, newbuf_len - prefix_len, fmt, saved_args);
       assert(ret >= 0, "Log message buffer issue");
       puts<Level>(newbuf);
       FREE_C_HEAP_ARRAY(char, newbuf);
--- a/hotspot/src/share/vm/utilities/ostream.cpp	Fri Jan 29 16:25:10 2016 -0800
+++ b/hotspot/src/share/vm/utilities/ostream.cpp	Fri Jan 29 20:57:09 2016 -0500
@@ -1099,7 +1099,7 @@
 void logStream::write(const char* s, size_t len) {
   if (len > 0 && s[len - 1] == '\n') {
     _current_line.write(s, len - 1);
-    _log_func(_current_line.as_string());
+    _log_func("%s", _current_line.as_string());
     _current_line.reset();
   } else {
     _current_line.write(s, len);
--- a/hotspot/src/share/vm/utilities/ostream.hpp	Fri Jan 29 16:25:10 2016 -0800
+++ b/hotspot/src/share/vm/utilities/ostream.hpp	Fri Jan 29 20:57:09 2016 -0500
@@ -27,6 +27,7 @@
 
 #include "memory/allocation.hpp"
 #include "runtime/timer.hpp"
+#include "utilities/globalDefinitions.hpp"
 
 class GCId;
 DEBUG_ONLY(class ResourceMark;)
@@ -249,7 +250,7 @@
 class logStream : public outputStream {
 private:
   stringStream _current_line;
-  void (*_log_func)(const char* fmt, ...);
+  void (*_log_func)(const char* fmt, ...) ATTRIBUTE_PRINTF(1, 2);
 public:
   void write(const char* s, size_t len);
   logStream(void (*log_func)(const char* fmt, ...)) : _log_func(log_func) {}