[core][apps] Some refactoring per clang-reported warnings (#3019).

.github/workflows/cxx11-win.yaml

@@ -17,8 +17,8 @@ jobs:
     - name: configure
       run: |
         md _build && cd _build
-        cmake ../ -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=OFF -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON
+        cmake ../ -DENABLE_STDCXX_SYNC=ON -DENABLE_ENCRYPTION=OFF -DENABLE_UNITTESTS=ON -DENABLE_BONDING=ON -DUSE_CXX_STD=c++11
     - name: build
-      run: cd _build && cmake --build ./ --config Release
+      run: cd _build && cmake --build ./ --config Release --verbose
     - name: test
       run: cd _build && ctest -E "TestIPv6.v6_calls_v4|TestConnectionTimeout.BlockingLoop" --extra-verbose -C Release

srtcore/congctl.cpp

@@ -595,7 +595,7 @@ class FileCC : public SrtCongestionControlBase
             {
                 m_dPktSndPeriod = m_dCWndSize / (m_parent->SRTT() + m_iRCInterval);
                 HLOGC(cclog.Debug, log << "FileCC: CHKTIMER, SLOWSTART:OFF, sndperiod=" << m_dPktSndPeriod << "us AS wndsize/(RTT+RCIV) (wndsize="
-                    << setprecision(6) << m_dCWndSize << " RTT=" << m_parent->SRTT() << " RCIV=" << m_iRCInterval << ")");
+                    << m_dCWndSize << " RTT=" << m_parent->SRTT() << " RCIV=" << m_iRCInterval << ")");
             }
         }
         else

srtcore/handshake.cpp

@@ -300,7 +300,7 @@ std::string srt::SrtFlagString(int32_t flags)
 #define LEN(arr) (sizeof (arr)/(sizeof ((arr)[0])))
 
     std::string output;
-    static std::string namera[] = { "TSBPD-snd", "TSBPD-rcv", "haicrypt", "TLPktDrop", "NAKReport", "ReXmitFlag", "StreamAPI" };
+    static std::string namera[] = { "TSBPD-snd", "TSBPD-rcv", "haicrypt", "TLPktDrop", "NAKReport", "ReXmitFlag", "StreamAPI", "FilterCapable" };
 
     size_t i = 0;
     for (; i < LEN(namera); ++i)

srtcore/logging.h

@@ -54,7 +54,7 @@ written by
 { \
     srt_logging::LogDispatcher::Proxy log(logdes); \
     log.setloc(__FILE__, __LINE__, __FUNCTION__); \
-    const srt_logging::LogDispatcher::Proxy& log_prox SRT_ATR_UNUSED = args; \
+    { (void)(const srt_logging::LogDispatcher::Proxy&)(args); } \
 }
 
 // LOGF uses printf-like style formatting.
@@ -147,6 +147,7 @@ struct SRT_API LogDispatcher
     LogLevel::type level;
     static const size_t MAX_PREFIX_SIZE = 32;
     char prefix[MAX_PREFIX_SIZE+1];
+    size_t prefix_len;
     LogConfig* src_config;
 
     bool isset(int flg) { return (src_config->flags & flg) != 0; }
@@ -159,30 +160,30 @@ struct SRT_API LogDispatcher
         level(log_level),
         src_config(&config)
     {
-        // XXX stpcpy desired, but not enough portable
-        // Composing the exact prefix is not critical, so simply
-        // cut the prefix, if the length is exceeded
-
-        // See Logger::Logger; we know this has normally 2 characters,
-        // except !!FATAL!!, which has 9. Still less than 32.
-        // If the size of the FA name together with severity exceeds the size,
-        // just skip the former.
-        if (logger_pfx && strlen(prefix) + strlen(logger_pfx) + 1 < MAX_PREFIX_SIZE)
+        const size_t your_pfx_len = your_pfx ? strlen(your_pfx) : 0;
+        const size_t logger_pfx_len = logger_pfx ? strlen(logger_pfx) : 0;
+
+        if (logger_pfx && your_pfx_len + logger_pfx_len + 1 < MAX_PREFIX_SIZE)
         {
-#if defined(_MSC_VER) && _MSC_VER < 1900
-            _snprintf(prefix, MAX_PREFIX_SIZE, "%s:%s", your_pfx, logger_pfx);
-#else
-            snprintf(prefix, MAX_PREFIX_SIZE + 1, "%s:%s", your_pfx, logger_pfx);
-#endif
+            memcpy(prefix, your_pfx, your_pfx_len);
+            prefix[your_pfx_len] = ':';
+            memcpy(prefix + your_pfx_len + 1, logger_pfx, logger_pfx_len);
+            prefix[your_pfx_len + logger_pfx_len + 1] = '\0';
+            prefix_len = your_pfx_len + logger_pfx_len + 1;
+        }
+        else if (your_pfx)
+        {
+            // Prefix too long, so copy only your_pfx and only
+            // as much as it fits
+            size_t copylen = std::min(+MAX_PREFIX_SIZE, your_pfx_len);
+            memcpy(prefix, your_pfx, copylen);
+            prefix[copylen] = '\0';
+            prefix_len = copylen;
         }
         else
         {
-#ifdef _MSC_VER
-            strncpy_s(prefix, MAX_PREFIX_SIZE + 1, your_pfx, _TRUNCATE);
-#else
-            strncpy(prefix, your_pfx, MAX_PREFIX_SIZE);
-            prefix[MAX_PREFIX_SIZE] = '\0';
-#endif
+            prefix[0] = '\0';
+            prefix_len = 0;
         }
     }
 
@@ -338,9 +339,9 @@ struct LogDispatcher::Proxy
 
     ~Proxy()
     {
-        if ( that_enabled )
+        if (that_enabled)
         {
-            if ( (flags & SRT_LOGF_DISABLE_EOL) == 0 )
+            if ((flags & SRT_LOGF_DISABLE_EOL) == 0)
                 os << std::endl;
             that.SendLogLine(i_file, i_line, area, os.str());
         }
@@ -381,7 +382,7 @@ struct LogDispatcher::Proxy
             buf[len-1] = '\0';
         }
 
-        os << buf;
+        os.write(buf, len);
         return *this;
     }
 };
@@ -494,7 +495,7 @@ inline void LogDispatcher::SendLogLine(const char* file, int line, const std::st
     }
     else if ( src_config->log_stream )
     {
-        (*src_config->log_stream) << msg;
+        src_config->log_stream->write(msg.data(), msg.size());
         (*src_config->log_stream).flush();
     }
     src_config->unlock();

srtcore/queue.h

@@ -591,6 +591,10 @@ struct CMultiplexer
         , m_pRcvQueue(NULL)
         , m_pChannel(NULL)
         , m_pTimer(NULL)
+        , m_iPort(0)
+        , m_iIPversion(0)
+        , m_iRefCount(1)
+        , m_iID(-1)
     {
     }
 

srtcore/utilities.h

@@ -981,30 +981,13 @@ inline std::string FormatBinaryString(const uint8_t* bytes, size_t size)
     if ( size == 0 )
         return "";
 
-    //char buf[256];
     using namespace std;
 
     ostringstream os;
+    os << setfill('0') << setw(2) << hex << uppercase;
 
-    // I know, it's funny to use sprintf and ostringstream simultaneously,
-    // but " %02X" in iostream is: << " " << hex << uppercase << setw(2) << setfill('0') << VALUE << setw(1)
-    // Too noisy. OTOH ostringstream solves the problem of memory allocation
-    // for a string of unpredictable size.
-    //sprintf(buf, "%02X", int(bytes[0]));
-
-    os.fill('0');
-    os.width(2);
-    os.setf(ios::basefield, ios::hex);
-    os.setf(ios::uppercase);
-
-    //os << buf;
-    os << int(bytes[0]);
-
-
-    for (size_t i = 1; i < size; ++i)
+    for (size_t i = 0; i < size; ++i)
     {
-        //sprintf(buf, " %02X", int(bytes[i]));
-        //os << buf;
         os << int(bytes[i]);
     }
     return os.str();

testing/srt-test-live.cpp

@@ -301,7 +301,7 @@ extern "C" int SrtCheckGroupHook(void* , SRTSOCKET acpsock, int , const sockaddr
         size = sizeof gt;
         if (-1 != srt_getsockflag(acpsock, SRTO_GROUPTYPE, &gt, &size))
         {
-            if (gt < Size(gtypes))
+            if (size_t(gt) < Size(gtypes))
                 Verb(" type=", gtypes[gt], VerbNoEOL);
             else
                 Verb(" type=", int(gt), VerbNoEOL);

testing/srt-test-multiplex.cpp

@@ -76,7 +76,7 @@ struct MediumPair
     bytevector initial_portion;
     string name;
 
-    MediumPair(unique_ptr<Source> s, unique_ptr<Target> t): src(move(s)), tar(move(t)) {}
+    MediumPair(unique_ptr<Source> s, unique_ptr<Target> t): src(std::move(s)), tar(std::move(t)) {}
 
     void Stop()
     {
@@ -190,9 +190,9 @@ class MediaBase
     ///        are still meant to be delivered to @c tar
     MediumPair& Link(std::unique_ptr<Source> src, std::unique_ptr<Target> tar, bytevector&& initial_portion, string name, string thread_name)
     {
-        media.emplace_back(move(src), move(tar));
+        media.emplace_back(std::move(src), std::move(tar));
         MediumPair& med = media.back();
-        med.initial_portion = move(initial_portion);
+        med.initial_portion = std::move(initial_portion);
         med.name = name;
 
         // Ok, got this, so we can start transmission.
@@ -382,7 +382,7 @@ bool SelectAndLink(SrtModel& m, string id, bool mode_output, string& w_msg)
     }
 
     bytevector dummy_initial_portion;
-    g_media_base.Link(move(source), move(target), move(dummy_initial_portion), os.str(), thread_name);
+    g_media_base.Link(std::move(source), std::move(target), std::move(dummy_initial_portion), os.str(), thread_name);
 
     return true;
 }

testing/srt-test-relay.cpp

@@ -320,7 +320,7 @@ SrtMainLoop::SrtMainLoop(const string& srt_uri, bool input_echoback, const strin
         Verb() << "Setting up output: " << spec;
         unique_ptr<TargetMedium> m { new TargetMedium };
         m->Setup(Target::Create(spec));
-        m_output_media.push_back(move(m));
+        m_output_media.push_back(std::move(m));
     }
 
 
@@ -369,7 +369,7 @@ SrtMainLoop::SrtMainLoop(const string& srt_uri, bool input_echoback, const strin
         // Add SRT medium to output targets, and keep input medium empty.
         unique_ptr<TargetMedium> med { new TargetMedium };
         med->Setup(m_srt_relay.get());
-        m_output_media.push_back(move(med));
+        m_output_media.push_back(std::move(med));
     }
     else
     {

testing/testactivemedia.cpp

@@ -120,4 +120,20 @@ void TargetMedium::Runner()
     }
 }
 
+bool TargetMedium::Schedule(const MediaPacket& data)
+{
+    LOGP(applog.Debug, "TargetMedium::Schedule LOCK ... ");
+    std::lock_guard<std::mutex> lg(buffer_lock);
+    LOGP(applog.Debug, "TargetMedium::Schedule LOCKED - checking: running=", running, " interrupt=", ::transmit_int_state);
+    if (!running || ::transmit_int_state)
+    {
+        LOGP(applog.Debug, "TargetMedium::Schedule: not running, discarding packet");
+        return false;
+    }
+
+    LOGP(applog.Debug, "TargetMedium(", typeid(*med).name(), "): Schedule: [", data.payload.size(), "] CLIENT -> BUFFER");
+    buffer.push_back(data);
+    ready.notify_one();
+    return true;
+}
 

testing/testactivemedia.hpp

@@ -6,7 +6,6 @@
 #include <exception>
 #include <thread>
 #include <mutex>
-#include <atomic>
 #include <condition_variable>
 
 #include "testmedia.hpp"
@@ -29,7 +28,7 @@ struct Medium
     std::mutex buffer_lock;
     std::thread thr;
     std::condition_variable ready;
-    std::atomic<bool> running = {false};
+    srt::sync::atomic<bool> running {false};
     std::exception_ptr xp; // To catch exception thrown by a thread
 
     virtual void Runner() = 0;
@@ -147,22 +146,7 @@ struct TargetMedium: Medium<Target>
 {
     void Runner() override;
 
-    bool Schedule(const MediaPacket& data)
-    {
-        LOGP(applog.Debug, "TargetMedium::Schedule LOCK ... ");
-        std::lock_guard<std::mutex> lg(buffer_lock);
-        LOGP(applog.Debug, "TargetMedium::Schedule LOCKED - checking: running=", running, " interrupt=", ::transmit_int_state);
-        if (!running || ::transmit_int_state)
-        {
-            LOGP(applog.Debug, "TargetMedium::Schedule: not running, discarding packet");
-            return false;
-        }
-
-        LOGP(applog.Debug, "TargetMedium(", typeid(*med).name(), "): Schedule: [", data.payload.size(), "] CLIENT -> BUFFER");
-        buffer.push_back(data);
-        ready.notify_one();
-        return true;
-    }
+    bool Schedule(const MediaPacket& data);
 
     void Clear()
     {

testing/testmedia.cpp

@@ -20,7 +20,6 @@
 #include <map>
 #include <chrono>
 #include <thread>
-#include <atomic>
 #include <srt.h>
 #if !defined(_WIN32)
 #include <sys/ioctl.h>
@@ -50,8 +49,8 @@ using srt_logging::SockStatusStr;
 using srt_logging::MemberStatusStr;
 #endif
 
-std::atomic<bool> transmit_throw_on_interrupt {false};
-std::atomic<bool> transmit_int_state {false};
+srt::sync::atomic<bool> transmit_throw_on_interrupt {false};
+srt::sync::atomic<bool> transmit_int_state {false};
 int transmit_bw_report = 0;
 unsigned transmit_stats_report = 0;
 size_t transmit_chunk_size = SRT_LIVE_DEF_PLSIZE;
@@ -348,7 +347,7 @@ void SrtCommon::InitParameters(string host, string path, map<string,string> par)
                 }
 
                 cc.token = token++;
-                m_group_nodes.push_back(move(cc));
+                m_group_nodes.push_back(std::move(cc));
             }
 
             par.erase("type");
@@ -3042,7 +3041,7 @@ extern unique_ptr<Base> CreateMedium(const string& uri)
     }
 
     if (ptr)
-        ptr->uri = move(u);
+        ptr->uri = std::move(u);
     return ptr;
 }
 

testing/testmedia.hpp

@@ -15,7 +15,8 @@
 #include <map>
 #include <stdexcept>
 #include <deque>
-#include <atomic>
+
+#include <sync.h> // use srt::sync::atomic instead of std::atomic for the sake of logging
 
 #include "apputil.hpp"
 #include "statswriter.hpp"
@@ -25,7 +26,7 @@
 
 extern srt_listen_callback_fn* transmit_accept_hook_fn;
 extern void* transmit_accept_hook_op;
-extern std::atomic<bool> transmit_int_state;
+extern srt::sync::atomic<bool> transmit_int_state;
 
 extern std::shared_ptr<SrtStatsWriter> transmit_stats_writer;
 

testing/testmediabase.hpp

@@ -11,17 +11,17 @@
 #ifndef INC_SRT_COMMON_TRANMITBASE_HPP
 #define INC_SRT_COMMON_TRANMITBASE_HPP
 
-#include <atomic>
 #include <string>
 #include <memory>
 #include <vector>
 #include <iostream>
 #include <stdexcept>
 
+#include "sync.h"
 #include "uriparser.hpp"
 
 typedef std::vector<char> bytevector;
-extern std::atomic<bool> transmit_throw_on_interrupt;
+extern srt::sync::atomic<bool> transmit_throw_on_interrupt;
 extern int transmit_bw_report;
 extern unsigned transmit_stats_report;
 extern size_t transmit_chunk_size;