szaszm commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r635299532



##########
File path: libminifi/include/core/logging/Logger.h
##########
@@ -68,13 +69,32 @@ inline T conditional_conversion(T t) {
 }
 
 template<typename ... Args>
-inline std::string format_string(char const* format_str, Args&&... args) {
-  char buf[LOG_BUFFER_SIZE];
-  std::snprintf(buf, LOG_BUFFER_SIZE, format_str, 
conditional_conversion(std::forward<Args>(args))...);
-  return std::string(buf);
+inline std::string format_string(int max_size, char const* format_str, 
Args&&... args) {
+  // try to use static buffer
+  char buf[LOG_BUFFER_SIZE + 1];
+  int result = std::snprintf(buf, LOG_BUFFER_SIZE + 1, format_str, 
conditional_conversion(std::forward<Args>(args))...);
+  if (result < 0) {
+    return std::string("Error while formatting log message");
+  }
+  if (result <= LOG_BUFFER_SIZE) {
+    // static buffer was large enough
+    return std::string(buf, result);
+  }
+  if (max_size >= 0 && max_size <= LOG_BUFFER_SIZE) {
+    // static buffer was already larger than allowed, use the filled buffer
+    return std::string(buf, LOG_BUFFER_SIZE);
+  }
+  // try to use dynamic buffer
+  size_t dynamic_buffer_size = max_size < 0 ? result : std::min(result, 
max_size);
+  std::vector<char> buffer(dynamic_buffer_size + 1);  // extra '\0' character
+  result = std::snprintf(buffer.data(), buffer.size(), format_str, 
conditional_conversion(std::forward<Args>(args))...);

Review comment:
       Since the size is already known at this time, we could allocate the 
memory in `std::string` directly. At least if you don't mind `const_cast`ing 
`buffer.data()` until C++17. 
https://en.cppreference.com/w/cpp/string/basic_string/data




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to