Merge #2547: [Util] Replace LogPrintf (but not LogPrint) macro with r…

…egular function

14f2239 [Util] Replace LogPrintf (but not LogPrint) macro with regular function (random-zebra)

Pull request description:

  This commit combines together bitcoin#14209 [MarcoFalke] and bitcoin#17218 [jkczyz]

  - The first one replaces `LogPrintf` and `LogPrint` macros with functions:

    > It is not possible to run the full test suite when configured with --enable-lcov, since logging is disabled currently so that "unnecessary branches are not analyzed". (See c8914b9)
    >
    > Fix this instead by replacing the macros with functions.

  - The second one restores the `LogPrint` macro:

    > Calling LogPrint with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining LogPrint as a macro prevents this unnecessary expression evaluation.
    >
    > This is a partial revert of 14209. The decision to revert is discussed in 16688, which adds verbose logging for validation event notification.

ACKs for top commit:
  furszy:
    ACK 14f2239
  Fuzzbawls:
    ACK 14f2239

Tree-SHA512: bbbe475aae784e18acaf8f4f1eae8f6114f9ddb5eff460225fd0a1d40f999826bac99fb63f1359d71d03c6694d9c56fa697bfdee546188fe4fdf1a2f3de3eb22

src/logging.h

@@ -137,28 +137,27 @@ std::vector<CLogCategoryActive> ListActiveLogCategories();
 /** Return true if str parses as a log category and set the flag */
 bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str);
 
-/** Get format string from VA_ARGS for error reporting */
-template<typename... Args> std::string FormatStringFromLogArgs(const char *fmt, const Args&... args) { return fmt; }
-
 // Be conservative when using LogPrintf/error or other things which
 // unconditionally log to debug.log! It should not be the case that an inbound
 // peer can fill up a user's disk with debug.log entries.
 
-#define LogPrintf(...) do {                                                         \
-    if(g_logger->Enabled()) {                                                       \
-        std::string _log_msg_; /* Unlikely name to avoid shadowing variables */     \
-        try {                                                                       \
-            _log_msg_ = tfm::format(__VA_ARGS__);                                   \
-        } catch (tinyformat::format_error &fmterr) {                                     \
-            /* Original format string will have newline so don't add one here */    \
-            _log_msg_ = "Error \"" + std::string(fmterr.what()) +                        \
-                        "\" while formatting log message: " +                       \
-                        FormatStringFromLogArgs(__VA_ARGS__);                       \
-        }                                                                           \
-        g_logger->LogPrintStr(_log_msg_);                                           \
-    }                                                                               \
-} while(0)
+template <typename... Args>
+static inline void LogPrintf(const char* fmt, const Args&... args)
+{
+    if (g_logger->Enabled()) {
+        std::string log_msg;
+        try {
+            log_msg = tfm::format(fmt, args...);
+        } catch (tinyformat::format_error& fmterr) {
+            /* Original format string will have newline so don't add one here */
+            log_msg = "Error \"" + std::string(fmterr.what()) + "\" while formatting log message: " + fmt;
+        }
+        g_logger->LogPrintStr(log_msg);
+    }
+}
 
+// Use a macro instead of a function for conditional logging to prevent
+// evaluating arguments when logging for the category is not enabled.
 #define LogPrint(category, ...) do {                                                \
     if (LogAcceptCategory((category))) {                                            \
         LogPrintf(__VA_ARGS__);                                                     \