Add new log macros and support disabling old ones (#3107)

* Refactor log macros to add new macros and allow not use old ones Signed-off-by: jparisu <[email protected]> * Fix trailing ; after calls Signed-off-by: jparisu <[email protected]> * change logError for EPROSIMA_LOG_ERROR Signed-off-by: jparisu <[email protected]> * change logWarning for EPROSIMA_LOG_WARNING Signed-off-by: jparisu <[email protected]> * change logInfo for EPROSIMA_LOG_INFO Signed-off-by: jparisu <[email protected]> * Add tests for macros Signed-off-by: jparisu <[email protected]> * apply suggestions Signed-off-by: jparisu <[email protected]> * apply macro name change Signed-off-by: jparisu <[email protected]> * fix typos Signed-off-by: jparisu <[email protected]> * uncrustify Signed-off-by: jparisu <[email protected]> * fix ; in RTPSParImpl Signed-off-by: jparisu <[email protected]> * Document feature in version.md after rebase Signed-off-by: jparisu <[email protected]> Signed-off-by: jparisu <[email protected]>
eProsima · Dec 1, 2022 · 1535aef · 1535aef
1 parent 6c14a04
commit 1535aef
Show file tree

Hide file tree

Showing 179 changed files with 5,948 additions and 4,537 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -303,13 +303,22 @@ option(LOG_NO_WARNING "Do not compile Warning Log level" OFF)
 
 option(LOG_NO_ERROR "Do not compile Error Log level" OFF)
 
+option(ENABLE_OLD_LOG_MACROS "Compile logInfo, logWarning and logError macros" ON)
+
 cmake_dependent_option(
     FASTDDS_ENFORCE_LOG_INFO
     "The LOG_NO_INFO option must be enforced regardless of selected configuration"
     OFF
     "NOT LOG_NO_INFO"
     OFF)
 
+# This name changes are required because ON is not considered as a C++ option, instead 1 is required
+if(ENABLE_OLD_LOG_MACROS)
+    set(ENABLE_OLD_LOG_MACROS_ 1)
+else()
+    set(ENABLE_OLD_LOG_MACROS_ 0)
+endif()
+
 if(LOG_NO_INFO)
     set(HAVE_LOG_NO_INFO 1)
 else()

diff --git a/examples/cpp/dds/HelloWorldExampleTCP/HelloWorldPublisher.cpp b/examples/cpp/dds/HelloWorldExampleTCP/HelloWorldPublisher.cpp
@@ -182,7 +182,7 @@ void HelloWorldPublisher::runThread(
         {
             if (publish(false))
             {
-                //logError(HW, "SENT " <<  hello_.index());
+                //EPROSIMA_LOG_ERROR(HW, "SENT " <<  hello_.index());
                 std::cout << "[RTCP] Message: " << hello_.message() << " with index: "
                           << hello_.index() << " SENT" << std::endl;
             }

diff --git a/include/fastdds/dds/core/LoanableSequence.hpp b/include/fastdds/dds/core/LoanableSequence.hpp
@@ -114,7 +114,7 @@ class LoanableSequence : public LoanableTypedCollection<T, _NonConstEnabler>
     {
         if (elements_ && !has_ownership_)
         {
-            logWarning(SUBSCRIBER, "Sequence destroyed with active loan");
+            EPROSIMA_LOG_WARNING(SUBSCRIBER, "Sequence destroyed with active loan");
             return;
         }
 

diff --git a/include/fastdds/dds/core/condition/Condition.hpp b/include/fastdds/dds/core/condition/Condition.hpp
@@ -48,7 +48,7 @@ class Condition
      */
     RTPS_DllAPI virtual bool get_trigger_value() const
     {
-        logWarning(CONDITION, "get_trigger_value public member function not implemented");
+        EPROSIMA_LOG_WARNING(CONDITION, "get_trigger_value public member function not implemented");
         return false; // TODO return trigger value
     }
 

diff --git a/include/fastdds/dds/log/Log.hpp b/include/fastdds/dds/log/Log.hpp
@@ -36,13 +36,32 @@
 
 // Logging API:
 
+// EPROSIMA LOG MACROS
+//! Logs an info message. Disable it through Log::SetVerbosity, define LOG_NO_INFO, or being in a release branch
+#define EPROSIMA_LOG_INFO(cat, msg) EPROSIMA_LOG_INFO_IMPL_(cat, msg)
+//! Logs a warning. Disable reporting through Log::SetVerbosity or define LOG_NO_WARNING
+#define EPROSIMA_LOG_WARNING(cat, msg) EPROSIMA_LOG_WARNING_IMPL_(cat, msg)
+//! Logs an error. Disable reporting through define LOG_NO_ERROR
+#define EPROSIMA_LOG_ERROR(cat, msg) EPROSIMA_LOG_ERROR_IMPL_(cat, msg)
+
+#if ENABLE_OLD_LOG_MACROS_
+// Compile old eProsima macros for compatibility shake.
+// However, these macros will be deprecated in future releases, so please do not use them.
+
 //! Logs an info message. Disable it through Log::SetVerbosity, define LOG_NO_INFO, or being in a release branch
 #define logInfo(cat, msg) logInfo_(cat, msg)
 //! Logs a warning. Disable reporting through Log::SetVerbosity or define LOG_NO_WARNING
 #define logWarning(cat, msg) logWarning_(cat, msg)
 //! Logs an error. Disable reporting through define LOG_NO_ERROR
 #define logError(cat, msg) logError_(cat, msg)
 
+//! Old internal macros. Just kept them in case some crazy bastard thoughtlessly used them
+#define logInfo_(cat, msg) EPROSIMA_LOG_INFO_IMPL_(cat, msg);
+#define logWarning_(cat, msg) EPROSIMA_LOG_WARNING_IMPL_(cat, msg);
+#define logError_(cat, msg) EPROSIMA_LOG_ERROR_IMPL_(cat, msg);
+
+#endif  // ENABLE_OLD_LOG_MACROS_
+
 namespace eprosima {
 namespace fastdds {
 namespace dds {
@@ -142,9 +161,12 @@ class Log
 
     /**
      * Not recommended to call this method directly! Use the following macros:
-     *  * logInfo(cat, msg);
-     *  * logWarning(cat, msg);
-     *  * logError(cat, msg);
+     *  * EPROSIMA_LOG_INFO(cat, msg);
+     *  * EPROSIMA_LOG_WARNING(cat, msg);
+     *  * EPROSIMA_LOG_ERROR(cat, msg);
+     *
+     * @todo this method takes 2 mutexes (same mutex) internally.
+     * This is a very high sensible point of the code and it should be refactored to be as efficient as possible.
      */
     RTPS_DllAPI static void QueueLog(
             const std::string& message,
@@ -205,32 +227,64 @@ class LogConsumer
 #define __func__ __FUNCTION__
 #endif // if defined(WIN32)
 
+/********************
+* Implementation of the log macros depending on the defined macros:
+* HAVE_LOG_NO_<level> disable completly a verbosity level
+* _INTERNALDEBUG || __INTERNALDEBUG  force to compile the log macro call even when it would not be added to queue
+* EPROSIMA_LOG_INFO_IMPL_ would only be compiled if HAVE_LOG_NO_INFO is OFF and
+* - FASTDDS_ENFORCE_LOG_INFO or (DEBUG and INTERNALDEBUG) are defined
+*
+* There are 3 implementations for each level:
+* 1. Compile and add log to queue
+* 2. Compile but do not add it to queue (with INTERNALDEBUG)
+* 3. Do not compile
+*
+* Every macro (with implementation) occurs inside a code block so after call every internal variable is destroyed.
+* Every macro declared has a do while(0).
+* This will not generate an assembler instruction and forces the user of the macro to use ";" after calling it.
+* https://gcc.gnu.org/onlinedocs/cpp/Swallowing-the-Semicolon.html
+* NOTE: some compilation cases do not use do while loop and so they do not force ";".
+* It is a risk that a user takes in exchange of a perfect way of non generating code in such cases.
+********************/
+
+/*********
+* ERROR *
+*********/
 // Name of variables inside macros must be unique, or it could produce an error with external variables
 #if !HAVE_LOG_NO_ERROR
-#define logError_(cat, msg)                                                                                            \
-    {                                                                                                                  \
+
+#define EPROSIMA_LOG_ERROR_IMPL_(cat, msg)                                                                                 \
+    do {                                                                                                               \
         using namespace eprosima::fastdds::dds;                                                                        \
         std::stringstream fastdds_log_ss_tmp__;                                                                        \
         fastdds_log_ss_tmp__ << msg;                                                                                   \
         Log::QueueLog(fastdds_log_ss_tmp__.str(), Log::Context{__FILE__, __LINE__, __func__, #cat}, Log::Kind::Error); \
-    }
+    } while (0)
+
 #elif (__INTERNALDEBUG || _INTERNALDEBUG)
-#define logError_(cat, msg)                                     \
-    {                                                           \
+
+#define EPROSIMA_LOG_ERROR_IMPL_(cat, msg)                          \
+    do {                                                        \
         auto fastdds_log_lambda_tmp__ = [&]()                   \
                 {                                               \
                     std::stringstream fastdds_log_ss_tmp__;     \
                     fastdds_log_ss_tmp__ << msg;                \
                 };                                              \
         (void)fastdds_log_lambda_tmp__;                         \
-    }
+    } while (0)
 #else
-#define logError_(cat, msg)
+
+#define EPROSIMA_LOG_ERROR_IMPL_(cat, msg)
+
 #endif // ifndef LOG_NO_ERROR
 
+/***********
+* WARNING *
+***********/
 #if !HAVE_LOG_NO_WARNING
-#define logWarning_(cat, msg)                                                                                       \
-    {                                                                                                               \
+
+#define EPROSIMA_LOG_WARNING_IMPL_(cat, msg)                                                                            \
+    do {                                                                                                            \
         using namespace eprosima::fastdds::dds;                                                                     \
         if (Log::GetVerbosity() >= Log::Kind::Warning)                                                              \
         {                                                                                                           \
@@ -239,28 +293,37 @@ class LogConsumer
             Log::QueueLog(                                                                                          \
                 fastdds_log_ss_tmp__.str(), Log::Context{__FILE__, __LINE__, __func__, #cat}, Log::Kind::Warning);  \
         }                                                                                                           \
-    }
+    } while (0)
+
 #elif (__INTERNALDEBUG || _INTERNALDEBUG)
-#define logWarning_(cat, msg)                                   \
-    {                                                           \
+
+#define EPROSIMA_LOG_WARNING_IMPL_(cat, msg)                        \
+    do {                                                        \
         auto fastdds_log_lambda_tmp__ = [&]()                   \
                 {                                               \
                     std::stringstream fastdds_log_ss_tmp__;     \
                     fastdds_log_ss_tmp__ << msg;                \
                 };                                              \
         (void)fastdds_log_lambda_tmp__;                         \
-    }
+    } while (0)
+
 #else
-#define logWarning_(cat, msg)
+
+#define EPROSIMA_LOG_WARNING_IMPL_(cat, msg)
+
 #endif // ifndef LOG_NO_WARNING
 
+/********
+* INFO *
+********/
 // Allow multiconfig platforms like windows to disable info queueing on Release and other non-debug configs
 #if !HAVE_LOG_NO_INFO &&  \
     (defined(FASTDDS_ENFORCE_LOG_INFO) || \
     ((defined(__INTERNALDEBUG) || defined(_INTERNALDEBUG)) && (defined(_DEBUG) || defined(__DEBUG) || \
     !defined(NDEBUG))))
-#define logInfo_(cat, msg)                                                                              \
-    {                                                                                                   \
+
+#define EPROSIMA_LOG_INFO_IMPL_(cat, msg)                                                                   \
+    do {                                                                                                \
         using namespace eprosima::fastdds::dds;                                                         \
         if (Log::GetVerbosity() >= Log::Kind::Info)                                                     \
         {                                                                                               \
@@ -269,19 +332,24 @@ class LogConsumer
             Log::QueueLog(fastdds_log_ss_tmp__.str(), Log::Context{__FILE__, __LINE__, __func__, #cat}, \
                     Log::Kind::Info);                                                                   \
         }                                                                                               \
-    }
+    } while (0)
+
 #elif (__INTERNALDEBUG || _INTERNALDEBUG)
-#define logInfo_(cat, msg)                                  \
-    {                                                       \
+
+#define EPROSIMA_LOG_INFO_IMPL_(cat, msg)                       \
+    do {                                                    \
         auto fastdds_log_lambda_tmp__ = [&]()               \
                 {                                           \
                     std::stringstream fastdds_log_ss_tmp__; \
                     fastdds_log_ss_tmp__ << msg;            \
                 };                                          \
         (void)fastdds_log_lambda_tmp__;                     \
-    }
+    } while (0)
+
 #else
-#define logInfo_(cat, msg)
+
+#define EPROSIMA_LOG_INFO_IMPL_(cat, msg)
+
 #endif // ifndef LOG_NO_INFO
 
 

diff --git a/include/fastdds/rtps/common/Locator.h b/include/fastdds/rtps/common/Locator.h
@@ -461,7 +461,7 @@ inline std::istream& operator >>(
                 if (addresses.first.empty())
                 {
                     loc.kind = LOCATOR_KIND_INVALID;
-                    logWarning(LOCATOR, "Error deserializing Locator");
+                    EPROSIMA_LOG_WARNING(LOCATOR, "Error deserializing Locator");
                     return input;
                 }
                 address = *addresses.first.begin();
@@ -473,7 +473,7 @@ inline std::istream& operator >>(
                 if (addresses.second.empty())
                 {
                     loc.kind = LOCATOR_KIND_INVALID;
-                    logWarning(LOCATOR, "Error deserializing Locator");
+                    EPROSIMA_LOG_WARNING(LOCATOR, "Error deserializing Locator");
                     return input;
                 }
                 address = *addresses.second.begin();
@@ -490,7 +490,7 @@ inline std::istream& operator >>(
         catch (std::ios_base::failure& )
         {
             loc.kind = LOCATOR_KIND_INVALID;
-            logWarning(LOCATOR, "Error deserializing Locator");
+            EPROSIMA_LOG_WARNING(LOCATOR, "Error deserializing Locator");
         }
 
         input.exceptions(excp_mask);

diff --git a/include/fastdds/rtps/common/LocatorList.hpp b/include/fastdds/rtps/common/LocatorList.hpp
@@ -473,7 +473,7 @@ inline std::istream& operator >>(
         catch (std::ios_base::failure& )
         {
             locList.clear();
-            logWarning(LOCATOR_LIST, "Error deserializing LocatorList");
+            EPROSIMA_LOG_WARNING(LOCATOR_LIST, "Error deserializing LocatorList");
         }
 
         input.exceptions(excp_mask);

diff --git a/include/fastdds/rtps/common/PortParameters.h b/include/fastdds/rtps/common/PortParameters.h
@@ -74,7 +74,7 @@ class PortParameters
 
         if (port > 65535)
         {
-            logError(RTPS, "Calculated port number is too high. Probably the domainId is over 232 "
+            EPROSIMA_LOG_ERROR(RTPS, "Calculated port number is too high. Probably the domainId is over 232 "
                     << "or portBase is too high.");
             std::cout << "Calculated port number is too high. Probably the domainId is over 232 "
                       << "or portBase is too high." << std::endl;
@@ -100,7 +100,7 @@ class PortParameters
 
         if (port > 65535)
         {
-            logError(RTPS, "Calculated port number is too high. Probably the domainId is over 232, there are "
+            EPROSIMA_LOG_ERROR(RTPS, "Calculated port number is too high. Probably the domainId is over 232, there are "
                     << "too much participants created or portBase is too high.");
             std::cout << "Calculated port number is too high. Probably the domainId is over 232, there are "
                       << "too much participants created or portBase is too high." << std::endl;

diff --git a/include/fastdds/rtps/common/RemoteLocators.hpp b/include/fastdds/rtps/common/RemoteLocators.hpp
@@ -224,7 +224,7 @@ inline std::istream& operator >>(
         {
             locList.unicast.clear();
             locList.multicast.clear();
-            logWarning(REMOTE_LOCATOR_LIST, "Error deserializing RemoteLocatorList");
+            EPROSIMA_LOG_WARNING(REMOTE_LOCATOR_LIST, "Error deserializing RemoteLocatorList");
         }
 
         input.exceptions(excp_mask);

diff --git a/include/fastdds/rtps/history/WriterHistory.h b/include/fastdds/rtps/history/WriterHistory.h
@@ -168,7 +168,8 @@ class WriterHistory : public rtps::History
     {
         if (mp_writer == nullptr || mp_mutex == nullptr)
         {
-            logError(RTPS_WRITER_HISTORY, "You need to create a Writer with this History before adding any changes");
+            EPROSIMA_LOG_ERROR(RTPS_WRITER_HISTORY,
+                    "You need to create a Writer with this History before adding any changes");
             return false;
         }
 

diff --git a/include/fastdds/rtps/security/logging/Logging.h b/include/fastdds/rtps/security/logging/Logging.h
@@ -243,21 +243,22 @@ bool Logging::compose_header(
                     MESSAGE,                                          \
                     std::string(CLASS ",") + __func__,                \
                     EXCEPTION);                                       \
-        } else {                                                      \
+        } \
+        else {                                                      \
             switch (LEVEL){                                           \
                 case LoggingLevel::EMERGENCY_LEVEL:                   \
                 case LoggingLevel::ALERT_LEVEL:                       \
                 case LoggingLevel::CRITICAL_LEVEL:                    \
                 case LoggingLevel::ERROR_LEVEL:                       \
-                    logError(SECURITY, MESSAGE);                      \
+                    EPROSIMA_LOG_ERROR(SECURITY, MESSAGE);                      \
                     break;                                            \
                 case LoggingLevel::WARNING_LEVEL:                     \
-                    logWarning(SECURITY, MESSAGE);                    \
+                    EPROSIMA_LOG_WARNING(SECURITY, MESSAGE);                    \
                     break;                                            \
                 case LoggingLevel::NOTICE_LEVEL:                      \
                 case LoggingLevel::INFORMATIONAL_LEVEL:               \
                 case LoggingLevel::DEBUG_LEVEL:                       \
-                    logInfo(SECURITY, MESSAGE);                       \
+                    EPROSIMA_LOG_INFO(SECURITY, MESSAGE);                       \
                     break;                                            \
             }                                                         \
         }                                                             \
@@ -276,7 +277,7 @@ bool Logging::compose_header(
         __FASTRTPS_MACRO_SELECTOR(__VA_ARGS__,  \
         __FASTRTPS_SECURITY_LOGGING,            \
         __FASTRTPS_SECURITY_LOGGING_EX,         \
-        _UNUSED)(__VA_ARGS__) )
+        _UNUSED)(__VA_ARGS__))
 
 #define EMERGENCY_SECURITY_LOGGING(...)     SECURITY_LOGGING(LoggingLevel::EMERGENCY_LEVEL, __VA_ARGS__)
 #define ALERT_SECURITY_LOGGING(...)         SECURITY_LOGGING(LoggingLevel::ALERT_LEVEL, __VA_ARGS__)

diff --git a/include/fastdds/rtps/writer/RTPSWriter.h b/include/fastdds/rtps/writer/RTPSWriter.h
@@ -550,7 +550,8 @@ class RTPSWriter
                 }
                 else
                 {
-                    logError(RTPS_WRITER, "Error sending fragment (" << change->sequenceNumber << ", " << frag << ")");
+                    EPROSIMA_LOG_ERROR(RTPS_WRITER,
+                            "Error sending fragment (" << change->sequenceNumber << ", " << frag << ")");
                     break;
                 }
             }
@@ -564,7 +565,7 @@ class RTPSWriter
             }
             else
             {
-                logError(RTPS_WRITER, "Error sending change " << change->sequenceNumber);
+                EPROSIMA_LOG_ERROR(RTPS_WRITER, "Error sending change " << change->sequenceNumber);
             }
         }
 

diff --git a/include/fastrtps/config.h.in b/include/fastrtps/config.h.in
@@ -77,6 +77,11 @@
 
 /* Log Macros */
 
+// Enable compilation for eProsima Log Macros
+#ifndef ENABLE_OLD_LOG_MACROS_
+#define ENABLE_OLD_LOG_MACROS_ @ENABLE_OLD_LOG_MACROS_@
+#endif /* ifndef ENABLE_OLD_LOG_MACROS_ */
+
 // Log Info
 #cmakedefine FASTDDS_ENFORCE_LOG_INFO
 #ifndef HAVE_LOG_NO_INFO