adamdebreceni commented on a change in pull request #1057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1057#discussion_r635310034
##########
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:
this thought crossed my mind, but this explicitly says that modifying
through the const overload is undefined behavior, I would wait until C++17
--
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
