Fix for dangling pointer in logging (#1093)

lynckia · Dec 15, 2017 · 316d6ec · 316d6ec
1 parent 25c1e6a
commit 316d6ec
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 47 deletions.
diff --git a/erizo/src/erizo/IceConnection.h b/erizo/src/erizo/IceConnection.h
@@ -117,8 +117,8 @@ class IceConnection : public LogContext {
   virtual std::string iceStateToString(IceState state) const;
 
  protected:
-  inline const char* toLog() {
-    return ("id: " + ice_config_.connection_id + ", " + printLogContext()).c_str();
+  inline std::string toLog() {
+    return "id: " + ice_config_.connection_id + ", " + printLogContext();
   }
 
  protected:

diff --git a/erizo/src/erizo/MediaStream.cpp b/erizo/src/erizo/MediaStream.cpp
@@ -252,7 +252,7 @@ void MediaStream::read(std::shared_ptr<DataPacket> packet) {
   // PROCESS RTCP
   RtpHeader *head = reinterpret_cast<RtpHeader*> (buf);
   RtcpHeader *chead = reinterpret_cast<RtcpHeader*> (buf);
-  uint32_t recvSSRC;
+  uint32_t recvSSRC = 0;
   if (!chead->isRtcp()) {
     recvSSRC = head->getSSRC();
   } else if (chead->packettype == RTCP_Sender_PT) {  // Sender Report

diff --git a/erizo/src/erizo/MediaStream.h b/erizo/src/erizo/MediaStream.h
@@ -123,8 +123,8 @@ class MediaStream: public MediaSink, public MediaSource, public FeedbackSink,
   bool isPipelineInitialized() { return pipeline_initialized_; }
   Pipeline::Ptr getPipeline() { return pipeline_; }
 
-  inline const char* toLog() {
-    return ("id: " + stream_id_ + ", " + printLogContext()).c_str();
+  inline std::string toLog() {
+    return "id: " + stream_id_ + ", " + printLogContext();
   }
 
  private:

diff --git a/erizo/src/erizo/Transport.h b/erizo/src/erizo/Transport.h
@@ -91,8 +91,8 @@ class Transport : public std::enable_shared_from_this<Transport>, public IceConn
 
   bool rtcp_mux_;
 
-  inline const char* toLog() {
-    return ("id: " + connection_id_ + ", " + printLogContext()).c_str();
+  inline std::string toLog() {
+    return "id: " + connection_id_ + ", " + printLogContext();
   }
 
   std::shared_ptr<Worker> getWorker() {

diff --git a/erizo/src/erizo/WebRtcConnection.h b/erizo/src/erizo/WebRtcConnection.h
@@ -143,8 +143,8 @@ class WebRtcConnection: public TransportListener, public LogContext,
   bool isSourceSSRC(uint32_t ssrc);
   bool isSinkSSRC(uint32_t ssrc);
 
-  inline const char* toLog() {
-    return ("id: " + connection_id_ + ", " + printLogContext()).c_str();
+  inline std::string toLog() {
+    return "id: " + connection_id_ + ", " + printLogContext();
   }
 
  private:

diff --git a/erizo/src/erizo/logger.h b/erizo/src/erizo/logger.h
@@ -28,6 +28,7 @@
 #include <map>
 #include <string>
 #include <utility>
+#include <type_traits>
 
 class LogContext {
  public:
@@ -69,68 +70,108 @@ log4cxx::LoggerPtr namespace::logger = log4cxx::Logger::getLogger(logName);
 char buffer[ELOG_MAX_BUFFER_SIZE]; \
 snprintf(buffer, ELOG_MAX_BUFFER_SIZE, fmt, ##args);
 
-// older versions of log4cxx don't support tracing
-#ifdef LOG4CXX_TRACE
-#define ELOG_TRACE2(logger, fmt, args...) \
-if (logger->isTraceEnabled()) { \
-  SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
-  LOG4CXX_TRACE(logger, __tmp); \
-}
-#else
 #define ELOG_TRACE2(logger, fmt, args...) \
-if (logger->isDebugEnabled()) { \
-  SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
-  LOG4CXX_DEBUG(logger, __tmp); \
-}
-#endif
+SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
+LOG4CXX_TRACE(logger, __tmp);
 
 #define ELOG_DEBUG2(logger, fmt, args...) \
-if (logger->isDebugEnabled()) { \
-  SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
-  LOG4CXX_DEBUG(logger, __tmp); \
-}
+SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
+LOG4CXX_DEBUG(logger, __tmp);
 
 #define ELOG_INFO2(logger, fmt, args...) \
-if (logger->isInfoEnabled()) { \
-  SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
-  LOG4CXX_INFO(logger, __tmp); \
-}
+SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
+LOG4CXX_INFO(logger, __tmp);
 
 #define ELOG_WARN2(logger, fmt, args...) \
-if (logger->isWarnEnabled()) { \
-  SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
-  LOG4CXX_WARN(logger, __tmp); \
-}
+SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
+LOG4CXX_WARN(logger, __tmp);
 
 #define ELOG_ERROR2(logger, fmt, args...) \
-if (logger->isErrorEnabled()) { \
-  SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
-  LOG4CXX_ERROR(logger, __tmp); \
-}
+SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
+LOG4CXX_ERROR(logger, __tmp);
 
 #define ELOG_FATAL2(logger, fmt, args...) \
-if (logger->isFatalEnabled()) { \
-  SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
-  LOG4CXX_FATAL(logger, __tmp); \
+SPRINTF_ELOG_MSG(__tmp, fmt, ##args); \
+LOG4CXX_FATAL(logger, __tmp);
+
+namespace detail {
+// Helper for forwarding correctly the object to be logged
+template <typename T>
+struct LogElementForwarder {
+  T operator()(T t) {
+    return t;
+  }
+};
+
+template <>
+struct LogElementForwarder<std::string> {
+  template <typename S>
+  const char* operator()(const S& t) {
+    return t.c_str();
+  }
+};
+}  // namespace detail
+
+#define DEFINE_ELOG_T(name, invoke) \
+template <typename Logger, typename... Args> \
+inline void name(const Logger&, const char*, Args...) __attribute__((always_inline)); \
+\
+template <typename Logger, typename... Args> \
+void name(const Logger& logger, const char* fmt, Args... args) { \
+  invoke(logger, fmt, detail::LogElementForwarder<typename std::decay<Args>::type>{}(args)...); \
+} \
+\
+template <typename Logger> \
+inline void name(const Logger&, const char*) __attribute__((always_inline)); \
+\
+template <typename Logger> \
+void name(const Logger& logger, const char* fmt) { \
+  invoke(logger, "%s", fmt); \
 }
 
+DEFINE_ELOG_T(ELOG_TRACET, ELOG_TRACE2)
+DEFINE_ELOG_T(ELOG_DEBUGT, ELOG_DEBUG2)
+DEFINE_ELOG_T(ELOG_INFOT, ELOG_INFO2)
+DEFINE_ELOG_T(ELOG_WARNT, ELOG_WARN2)
+DEFINE_ELOG_T(ELOG_ERRORT, ELOG_ERROR2)
+DEFINE_ELOG_T(ELOG_FATALT, ELOG_FATAL2)
 
+// older versions of log4cxx don't support tracing
+#ifdef LOG4CXX_TRACE
 #define ELOG_TRACE(fmt, args...) \
-ELOG_TRACE2(logger, fmt, ##args);
+if (logger->isTraceEnabled()) { \
+  ELOG_TRACET(logger, fmt, ##args); \
+}
+#else
+#define ELOG_TRACE(fmt, args...) \
+if (logger->isDebugEnabled()) { \
+  ELOG_DEBUGT(logger, fmt, ##args); \
+}
+#endif
 
 #define ELOG_DEBUG(fmt, args...) \
-ELOG_DEBUG2(logger, fmt, ##args);
+if (logger->isDebugEnabled()) { \
+  ELOG_DEBUGT(logger, fmt, ##args); \
+}
 
 #define ELOG_INFO(fmt, args...) \
-ELOG_INFO2(logger, fmt, ##args);
+if (logger->isInfoEnabled()) { \
+  ELOG_INFOT(logger, fmt, ##args); \
+}
 
 #define ELOG_WARN(fmt, args...) \
-ELOG_WARN2(logger, fmt, ##args);
+if (logger->isWarnEnabled()) { \
+  ELOG_WARNT(logger, fmt, ##args); \
+}
 
 #define ELOG_ERROR(fmt, args...) \
-ELOG_ERROR2(logger, fmt, ##args);
+if (logger->isErrorEnabled()) { \
+  ELOG_ERRORT(logger, fmt, ##args); \
+}
 
 #define ELOG_FATAL(fmt, args...) \
-ELOG_FATAL2(logger, fmt, ##args);
+if (logger->isFatalEnabled()) { \
+  ELOG_FATALT(logger, fmt, ##args); \
+}
 
 #endif  // ERIZO_SRC_ERIZO_LOGGER_H_