Fixed a few minor bugs and removed some code smells

daemon/browser.cpp

@@ -96,6 +96,8 @@ bool Browser::worker() {
           BOOST_LOG_TRIVIAL(debug)
               << "browser:: refreshing SAP source " << it->id;
           // annoucement, update last seen and announce period
+          auto last_update_ =
+              duration_cast<second_t>(steady_clock::now() - startup_).count();
           auto upd_source{*it};
           if ((last_update_ - upd_source.last_seen) != 0) {
             upd_source.announce_period = last_update_ - upd_source.last_seen;

daemon/browser.hpp

@@ -57,7 +57,6 @@ class Browser : public MDNSClient {
   Browser() = delete;
   Browser(const Browser&) = delete;
   Browser& operator=(const Browser&) = delete;
-  virtual ~Browser() { terminate(); };
 
   bool init() override;
   bool terminate() override;
@@ -68,17 +67,17 @@ class Browser : public MDNSClient {
 
  protected:
   // singleton, use create() to build
-  Browser(std::shared_ptr<Config> config)
+  explicit Browser(std::shared_ptr<Config> config) : MDNSClient(config){};
-      : MDNSClient(config), startup_(std::chrono::steady_clock::now()){};
 
   bool worker();
 
-  virtual void on_change_rtsp_source(const std::string& name,
+  void on_change_rtsp_source(const std::string& name,
                              const std::string& domain,
                              const RtspSource& source) override;
-  virtual void on_remove_rtsp_source(const std::string& name,
+  void on_remove_rtsp_source(const std::string& name,
                              const std::string& domain) override;
 
+ private:
   std::future<bool> res_;
   std::atomic_bool running_{false};
 
@@ -99,7 +98,7 @@ class Browser : public MDNSClient {
 
   SAP sap_{config_->get_sap_mcast_addr()};
   IGMP igmp_;
-  std::chrono::time_point<std::chrono::steady_clock> startup_;
+  std::chrono::time_point<std::chrono::steady_clock> startup_{std::chrono::steady_clock::now()};
   uint32_t last_update_{0}; /* seconds from daemon startup */
 };
 

daemon/config.cpp

@@ -75,7 +75,6 @@ std::shared_ptr<Config> Config::parse(const std::string& filename,
   if (ec) {
     config.sap_mcast_addr_ = "224.2.127.254";
   }
-  if (config.ptp_domain_ > 127)
   if (config.ptp_domain_ > 127)
     config.ptp_domain_ = 0;
 

daemon/config.hpp

@@ -35,7 +35,7 @@ class Config {
   /* attributes retrieved from config json */
   uint16_t get_http_port() const { return http_port_; };
   uint16_t get_rtsp_port() const { return rtsp_port_; };
-  const std::string get_http_base_dir() const { return http_base_dir_; };
+  const std::string& get_http_base_dir() const { return http_base_dir_; };
   int get_log_severity() const { return log_severity_; };
   uint32_t get_playout_delay() const { return playout_delay_; };
   uint32_t get_tic_frame_size_at_1fs() const { return tic_frame_size_at_1fs_; };
@@ -64,7 +64,7 @@ class Config {
   bool get_daemon_restart() const { return daemon_restart_; };
   bool get_driver_restart() const { return driver_restart_; };
   bool get_mdns_enabled() const { return mdns_enabled_; };
-  int get_interface_idx() { return interface_idx_; };
+  int get_interface_idx() const { return interface_idx_; };
   const std::string& get_ptp_status_script() const {
     return ptp_status_script_;
   }

daemon/driver_handler.cpp

@@ -54,7 +54,7 @@ void DriverHandler::send(enum MT_ALSA_msg_id id,
                          NetlinkClient& client,
                          uint8_t* buffer,
                          size_t data_size,
-                         const uint8_t* data) {
+                         const uint8_t* data) const {
   struct MT_ALSA_msg alsa_msg;
   memset(&alsa_msg, 0, sizeof(alsa_msg));
   alsa_msg.id = id;
@@ -143,7 +143,7 @@ bool DriverHandler::terminate(const Config& /* config */) {
 void DriverHandler::send_command(enum MT_ALSA_msg_id id,
                                  size_t data_size,
                                  const uint8_t* data) {
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock<std::mutex> lock{mutex_};
   if (data_size > max_payload) {
     on_command_error(id, DaemonErrc::send_invalid_size);
     return;

daemon/driver_handler.hpp

@@ -37,7 +37,7 @@ class DriverHandler {
   static constexpr size_t buffer_size =
       NLMSG_SPACE(max_payload) + sizeof(struct MT_ALSA_msg);
 
-  DriverHandler(){};
+  DriverHandler() = default;
   DriverHandler(const DriverHandler&) = delete;
   DriverHandler& operator=(const DriverHandler&) = delete;
   virtual ~DriverHandler(){};
@@ -67,7 +67,7 @@ class DriverHandler {
             NetlinkClient& client,
             uint8_t* buffer,
             size_t data_size,
-            const uint8_t* data);
+            const uint8_t* data) const;
   bool event_receiver();
 
   std::future<bool> res_;

daemon/driver_manager.cpp

@@ -75,7 +75,7 @@ bool DriverManager::init(const Config& config) {
     return false;
   }
 
-  sample_rate = config.get_sample_rate();
+  sample_rate_ = config.get_sample_rate();
 
   TPTPConfig ptp_config;
   ptp_config.ui8Domain = config.get_ptp_domain();
@@ -287,39 +287,41 @@ void DriverManager::on_event(enum MT_ALSA_msg_id id,
       break;
     case MT_ALSA_Msg_SetMasterOutputVolume:
       if (req_size == sizeof(int32_t)) {
-        memcpy(&output_volume, req, req_size);
+        memcpy(&output_volume_, req, req_size);
         BOOST_LOG_TRIVIAL(info)
-            << "driver_manager:: event SetMasterOutputVolume " << output_volume;
+            << "driver_manager:: event SetMasterOutputVolume "
+            << output_volume_;
       }
       resp_size = 0;
       break;
     case MT_ALSA_Msg_SetMasterOutputSwitch:
       if (req_size == sizeof(int32_t)) {
-        memcpy(&output_switch, req, req_size);
+        memcpy(&output_switch_, req, req_size);
         BOOST_LOG_TRIVIAL(info)
-            << "driver_manager:: event SetMasterOutputSwitch " << output_switch;
+            << "driver_manager:: event SetMasterOutputSwitch "
+            << output_switch_;
       }
       resp_size = 0;
       break;
     case MT_ALSA_Msg_SetSampleRate:
       if (req_size == sizeof(uint32_t)) {
-        memcpy(&sample_rate, req, req_size);
+        memcpy(&sample_rate_, req, req_size);
         BOOST_LOG_TRIVIAL(info)
-            << "driver_manager:: event SetSampleRate " << sample_rate;
+            << "driver_manager:: event SetSampleRate " << sample_rate_;
       }
       resp_size = 0;
       break;
     case MT_ALSA_Msg_GetMasterOutputVolume:
       resp_size = sizeof(int32_t);
-      memcpy(resp, &output_volume, resp_size);
+      memcpy(resp, &output_volume_, resp_size);
       BOOST_LOG_TRIVIAL(info)
-          << "driver_manager:: event GetMasterOutputVolume " << output_volume;
+          << "driver_manager:: event GetMasterOutputVolume " << output_volume_;
       break;
     case MT_ALSA_Msg_GetMasterOutputSwitch:
       resp_size = sizeof(int32_t);
-      memcpy(resp, &output_switch, resp_size);
+      memcpy(resp, &output_switch_, resp_size);
       BOOST_LOG_TRIVIAL(info)
-          << "driver_manager:: event GetMasterOutputSwitch " << output_switch;
+          << "driver_manager:: event GetMasterOutputSwitch " << output_switch_;
       break;
     default:
       BOOST_LOG_TRIVIAL(error) << "driver_manager:: unknown event "

daemon/driver_manager.hpp

@@ -53,13 +53,13 @@ class DriverManager : public DriverHandler {
   std::error_code get_number_of_inputs(int32_t& inputs);
   std::error_code get_number_of_outputs(int32_t& outputs);
 
-  int32_t get_current_output_volume() { return output_volume; };
+  int32_t get_current_output_volume() const { return output_volume_; };
-  int32_t get_current_output_switch() { return output_switch; };
+  int32_t get_current_output_switch() const { return output_switch_; };
-  uint32_t get_current_sample_rate() { return sample_rate; };
+  uint32_t get_current_sample_rate() const { return sample_rate_; };
 
  protected:
   // singleton, use create to build
-  DriverManager(){};
+  DriverManager() = default;
 
   // these are used in init/terminate
   std::error_code hello();
@@ -79,12 +79,13 @@ class DriverManager : public DriverHandler {
                 const uint8_t* req = nullptr) override;
   void on_event_error(enum MT_ALSA_msg_id id, std::error_code error) override;
 
+ private:
   std::error_code retcode_;
   uint8_t recv_data_[NLMSG_SPACE(max_payload)]{0};
 
-  int32_t output_volume{-20};
+  int32_t output_volume_{-20};
-  int32_t output_switch{0};
+  int32_t output_switch_{0};
-  uint32_t sample_rate{0};
+  uint32_t sample_rate_{0};
 };
 
 #endif

daemon/fake_driver_manager.cpp

@@ -120,7 +120,8 @@ std::error_code DriverManager::add_rtp_stream(
     uint64_t& stream_handle) {
   stream_handle = ++g_handle;
   handles_.insert(stream_handle);
-  BOOST_LOG_TRIVIAL(info) << "fake_driver_manager:: add RTP stream success handle "
+  BOOST_LOG_TRIVIAL(info)
+      << "fake_driver_manager:: add RTP stream success handle "
       << stream_handle;
   return std::error_code{};
 }
@@ -170,7 +171,8 @@ std::error_code DriverManager::set_playout_delay(int32_t delay) {
 
 std::error_code DriverManager::get_sample_rate(uint32_t& sample_rate) {
   sample_rate = sample_rate_;
-  BOOST_LOG_TRIVIAL(info) << "fake_driver_manager:: sample rate " << sample_rate;
+  BOOST_LOG_TRIVIAL(info) << "fake_driver_manager:: sample rate "
+                          << sample_rate;
   return std::error_code{};
 }
 

daemon/http_server.cpp

@@ -31,7 +31,7 @@
 using namespace httplib;
 
 static inline void set_headers(Response& res,
-                               const std::string content_type = "") {
+                               const std::string& content_type = "") {
   res.set_header("Access-Control-Allow-Methods",
                  "GET, POST, PUT, DELETE, OPTIONS");
   res.set_header("Access-Control-Allow-Origin", "*");
@@ -57,7 +57,6 @@ static inline int get_http_error_status(const std::error_code& code) {
 
 static inline std::string get_http_error_message(const std::error_code& code) {
   std::stringstream ss;
-  ;
   ss << "(" << code.category().name() << ") " << code.message();
   return ss.str();
 }

daemon/http_server.hpp

@@ -29,7 +29,7 @@
 class HttpServer {
  public:
   HttpServer() = delete;
-  HttpServer(std::shared_ptr<SessionManager> session_manager,
+  explicit HttpServer(std::shared_ptr<SessionManager> session_manager,
                       std::shared_ptr<Browser> browser,
                       std::shared_ptr<Config> config)
       : session_manager_(session_manager), browser_(browser), config_(config){};

daemon/igmp.hpp

@@ -41,11 +41,11 @@ class IGMP {
   bool join(const std::string& interface_ip, const std::string& mcast_ip) {
     uint32_t mcast_ip_addr =
         ip::address_v4::from_string(mcast_ip.c_str()).to_ulong();
-    std::lock_guard<std::mutex> lock(mutex);
+    std::scoped_lock<std::mutex> lock{mutex};
 
     auto it = mcast_ref.find(mcast_ip_addr);
     if (it != mcast_ref.end() && (*it).second > 0) {
-      mcast_ref[mcast_ip_addr]++;
+      (*it).second++;
       return true;
     }
 
@@ -76,14 +76,14 @@ class IGMP {
   bool leave(const std::string& interface_ip, const std::string& mcast_ip) {
     uint32_t mcast_ip_addr =
         ip::address_v4::from_string(mcast_ip.c_str()).to_ulong();
-    std::lock_guard<std::mutex> lock(mutex);
+    std::scoped_lock<std::mutex> lock{mutex};
 
     auto it = mcast_ref.find(mcast_ip_addr);
     if (it == mcast_ref.end() || (*it).second == 0) {
       return false;
     }
 
-    if (--mcast_ref[mcast_ip_addr] > 0) {
+    if (--(*it).second > 0) {
       return true;
     }
 

daemon/interface.cpp

@@ -127,7 +127,7 @@ std::pair<std::array<uint8_t, 6>, std::string> get_mac_from_arp_cache(
     std::vector<std::string> tokens;
 
     std::getline(stream, line);
-    if (line.find(ip)) {
+    if (line.find(ip) == std::string::npos) {
       continue;
     }
     boost::split(tokens, line, boost::is_any_of(" "), boost::token_compress_on);
@@ -139,8 +139,8 @@ std::pair<std::array<uint8_t, 6>, std::string> get_mac_from_arp_cache(
       int j = 0;
       bool check = false;
       for (auto const& item : vec) {
-        mac[j] = strtol(item.c_str(), NULL, 16);
+        mac[j] = strtol(item.c_str(), nullptr, 16);
-        check |= mac[j];
+        check = check | (mac[j] != 0);
         j++;
       }
       if (check) {

daemon/json.cpp

@@ -29,7 +29,7 @@
 #include "json.hpp"
 
 static inline std::string remove_undesired_chars(const std::string& s) {
-  std::regex html_regex("[^ A-Za-z0-9:~.,_/=%\()\\r\\n\\t\?#-]?");
+  std::regex html_regex("[^ A-Za-z0-9:~.,_/=%()\\r\\n\\t\?#-]?");
   return std::regex_replace(s, html_regex, "");
 }
 

daemon/mdns_client.cpp

@@ -87,7 +87,7 @@ void MDNSClient::resolve_callback(AvahiServiceResolver* r,
            (mdns.config_->get_interface_name() != "lo")) ||
           ((flags & AVAHI_LOOKUP_RESULT_LOCAL) &&
            (mdns.config_->get_interface_name() == "lo"))) {
-        std::lock_guard<std::mutex> lock(mdns.sources_res_mutex_);
+        std::scoped_lock<std::mutex> lock{mdns.sources_res_mutex_};
 
         /* process RTSP client in async task */
         mdns.sources_res_.emplace_back(std::async(
@@ -153,7 +153,7 @@ void MDNSClient::browse_callback(AvahiServiceBrowser* b,
             << avahi_strerror(avahi_client_errno(mdns.client_.get()));
       } else {
         /* add the resolver to the active pool */
-        mdns.active_resolvers.insert({name, domain});
+        mdns.active_resolvers.emplace(name, domain);
       }
       break;
 
@@ -244,7 +244,7 @@ bool MDNSClient::init() {
 
 void MDNSClient::process_results() {
 #ifdef _USE_AVAHI_
-  std::lock_guard<std::mutex> lock(sources_res_mutex_);
+  std::scoped_lock<std::mutex> lock{sources_res_mutex_};
   /* remove all completed results and populate remote sources list */
   sources_res_.remove_if([](auto& result) {
     if (!result.valid()) {
@@ -269,7 +269,7 @@ bool MDNSClient::terminate() {
     RtspClient::stop_all();
 #ifdef _USE_AVAHI_
     /* wait for all pending results and remove from list */
-    std::lock_guard<std::mutex> lock(sources_res_mutex_);
+    std::scoped_lock<std::mutex> lock{sources_res_mutex_};
     BOOST_LOG_TRIVIAL(info) << "mdns_client:: waiting for "
                             << sources_res_.size() << " RTSP clients";
     sources_res_.remove_if([](auto& result) {

daemon/mdns_client.hpp

@@ -43,7 +43,7 @@ class MDNSClient {
   MDNSClient() = delete;
   MDNSClient(const MDNSClient&) = delete;
   MDNSClient& operator=(const MDNSClient&) = delete;
-  virtual ~MDNSClient() { terminate(); };
+  virtual ~MDNSClient() = default;
 
   virtual bool init();
   virtual bool terminate();
@@ -55,22 +55,6 @@ class MDNSClient {
   virtual void on_remove_rtsp_source(const std::string& name,
                                      const std::string& domain){};
 
-  void process_results();
-  std::list<std::future<void> > sources_res_;
-  std::mutex sources_res_mutex_;
-
-  std::atomic_bool running_{false};
-  std::shared_ptr<Config> config_;
-
-#ifdef _USE_AVAHI_
-  /* order is important here */
-  std::unique_ptr<AvahiThreadedPoll, decltype(&avahi_threaded_poll_free)> poll_{
-      nullptr, &avahi_threaded_poll_free};
-  std::unique_ptr<AvahiClient, decltype(&avahi_client_free)> client_{
-      nullptr, &avahi_client_free};
-  std::unique_ptr<AvahiServiceBrowser, decltype(&avahi_service_browser_free)>
-      sb_{nullptr, &avahi_service_browser_free};
-
   static void resolve_callback(AvahiServiceResolver* r,
                                AvahiIfIndex interface,
                                AvahiProtocol protocol,
@@ -97,9 +81,26 @@ class MDNSClient {
                               AvahiClientState state,
                               void* userdata);
 
+  void process_results();
+
+  std::shared_ptr<Config> config_;
+
+ private:
+  std::list<std::future<void> > sources_res_;
+  std::mutex sources_res_mutex_;
+
+  std::atomic_bool running_{false};
+
+#ifdef _USE_AVAHI_
+  /* order is important here */
+  std::unique_ptr<AvahiThreadedPoll, decltype(&avahi_threaded_poll_free)> poll_{
+      nullptr, &avahi_threaded_poll_free};
+  std::unique_ptr<AvahiClient, decltype(&avahi_client_free)> client_{
+      nullptr, &avahi_client_free};
+  std::unique_ptr<AvahiServiceBrowser, decltype(&avahi_service_browser_free)>
+      sb_{nullptr, &avahi_service_browser_free};
   std::set<std::pair<std::string /*name*/, std::string /*domain */> >
       active_resolvers;
-
 #endif
 };
 

daemon/mdns_server.cpp

@@ -28,7 +28,7 @@
 #ifdef _USE_AVAHI_
 struct AvahiLockGuard {
   AvahiLockGuard() = delete;
-  AvahiLockGuard(AvahiThreadedPoll* poll) : poll_(poll) {
+  explicit AvahiLockGuard(AvahiThreadedPoll* poll) : poll_(poll) {
     if (poll_ != nullptr) {
       avahi_threaded_poll_lock(poll_);
     }

daemon/mdns_server.hpp

@@ -46,7 +46,7 @@ class MDNSServer {
   MDNSServer() = delete;
   MDNSServer(const MDNSServer&) = delete;
   MDNSServer& operator=(const MDNSServer&) = delete;
-  virtual ~MDNSServer() { terminate(); };
+  virtual ~MDNSServer() = default;
 
   virtual bool init();
   virtual bool terminate();
@@ -55,6 +55,16 @@ class MDNSServer {
   bool remove_service(const std::string& name);
 
  protected:
+  static void entry_group_callback(AvahiEntryGroup* g,
+                                   AvahiEntryGroupState state,
+                                   void* userdata);
+  static void client_callback(AvahiClient* c,
+                              AvahiClientState state,
+                              void* userdata);
+
+  bool create_services(AvahiClient* client);
+
+ private:
   std::atomic_bool running_{false};
   std::shared_ptr<SessionManager> session_manager_;
   std::shared_ptr<Config> config_;
@@ -72,14 +82,6 @@ class MDNSServer {
   std::unique_ptr<AvahiClient, decltype(&avahi_client_free)> client_{
       nullptr, &avahi_client_free};
 
-  static void entry_group_callback(AvahiEntryGroup* g,
-                                   AvahiEntryGroupState state,
-                                   void* userdata);
-  static void client_callback(AvahiClient* c,
-                              AvahiClientState state,
-                              void* userdata);
-
-  bool create_services(AvahiClient* client);
 #endif
 };
 

daemon/netlink.hpp

@@ -26,62 +26,55 @@
 template <typename Protocol>
 class nl_endpoint {
  private:
-  sockaddr_nl sockaddr{.nl_family = AF_NETLINK};
+  sockaddr_nl sockaddr_{.nl_family = AF_NETLINK};
 
  public:
   using protocol_type = Protocol;
   using data_type = boost::asio::detail::socket_addr_type;
 
   nl_endpoint() {
-    sockaddr.nl_groups = 0;
+    sockaddr_.nl_groups = 0;
-    sockaddr.nl_pid = getpid();
+    sockaddr_.nl_pid = getpid();
   }
 
   nl_endpoint(int group, int pid = getpid()) {
-    sockaddr.nl_groups = group;
+    sockaddr_.nl_groups = group;
-    sockaddr.nl_pid = pid;
+    sockaddr_.nl_pid = pid;
-  }
-
-  nl_endpoint(const nl_endpoint& other) { sockaddr = other.sockaddr; }
-
-  nl_endpoint& operator=(const nl_endpoint& other) {
-    sockaddr = other.sockaddr;
-    return *this;
   }
 
   protocol_type protocol() const { return protocol_type(); }
 
-  data_type* data() { return reinterpret_cast<struct sockaddr*>(&sockaddr); }
+  data_type* data() { return reinterpret_cast<struct sockaddr*>(&sockaddr_); }
 
   const data_type* data() const {
-    return reinterpret_cast<const struct sockaddr*>(&sockaddr);
+    return reinterpret_cast<const struct sockaddr*>(&sockaddr_);
   }
 
-  std::size_t size() const { return sizeof(sockaddr); }
+  std::size_t size() const { return sizeof(sockaddr_); }
 
   void resize(std::size_t size) { /* nothing we can do here */
   }
 
-  std::size_t capacity() const { return sizeof(sockaddr); }
+  std::size_t capacity() const { return sizeof(sockaddr_); }
 };
 
 class nl_protocol {
  public:
-  nl_protocol() { proto = 0; }
+  nl_protocol() : proto_(0) {}
 
-  explicit nl_protocol(int proto) { this->proto = proto; }
+  explicit nl_protocol(int proto) : proto_(proto) {}
 
   int type() const { return SOCK_RAW; }
 
-  int protocol() const { return proto; }
+  int protocol() const { return proto_; }
 
   int family() const { return PF_NETLINK; }
 
-  typedef nl_endpoint<nl_protocol> endpoint;
+  using endpoint = nl_endpoint<nl_protocol>;
-  typedef boost::asio::basic_raw_socket<nl_protocol> socket;
+  using socket = boost::asio::basic_raw_socket<nl_protocol>;
 
  private:
-  int proto;
+  int proto_;
 };
 
 #endif

daemon/rtsp_client.cpp

@@ -98,14 +98,13 @@ struct RtspActiveClientRemover {
         domain_(domain),
         wait_for_updates_(wait_for_updates) {
     if (stream_ != nullptr && wait_for_updates_) {
-      RtspClient::g_mutex.lock();
+      std::scoped_lock<std::mutex> lock{RtspClient::g_mutex};
       RtspClient::g_active_clients[{name_, domain_}] = stream_;
-      RtspClient::g_mutex.unlock();
     }
   }
   ~RtspActiveClientRemover() {
     if (stream_ != nullptr && wait_for_updates_) {
-      std::lock_guard<std::mutex> lock(RtspClient::g_mutex);
+      std::scoped_lock<std::mutex> lock{RtspClient::g_mutex};
       auto it = RtspClient::g_active_clients.find({name_, domain_});
       if (it != RtspClient::g_active_clients.end() && it->second == stream_) {
         RtspClient::g_active_clients.erase(it);
@@ -120,7 +119,8 @@ struct RtspActiveClientRemover {
   bool wait_for_updates_{false};
 };
 
-std::pair<bool, RtspSource> RtspClient::process(RtspClient::Observer callback,
+std::pair<bool, RtspSource> RtspClient::process(
+    const RtspClient::Observer& callback,
     const std::string& name,
     const std::string& domain,
     const std::string& path,
@@ -258,9 +258,9 @@ std::pair<bool, RtspSource> RtspClient::process(RtspClient::Observer callback,
           if (std::get<0>(res)) {
             /* if we find a valid announced source name we use it
              * otherwise we try from SDP file or we use the mDNS name */
-            auto path = std::get<4>(res);
+            const auto& lpath = std::get<4>(res);
             if (path.rfind("/by-name/") != std::string::npos) {
-              announced_name = path.substr(9);
+              announced_name = lpath.substr(9);
               BOOST_LOG_TRIVIAL(debug)
                   << "rtsp_client:: found announced name " << announced_name;
             }
@@ -279,7 +279,7 @@ std::pair<bool, RtspSource> RtspClient::process(RtspClient::Observer callback,
 }
 
 void RtspClient::stop(const std::string& name, const std::string& domain) {
-  std::lock_guard<std::mutex> lock(g_mutex);
+  std::scoped_lock<std::mutex> lock{g_mutex};
   auto it = g_active_clients.find({name, domain});
   if (it != g_active_clients.end()) {
     BOOST_LOG_TRIVIAL(info)
@@ -294,12 +294,12 @@ void RtspClient::stop(const std::string& name, const std::string& domain) {
 }
 
 bool RtspClient::is_active(const std::string& name, const std::string& domain) {
-  std::lock_guard<std::mutex> lock(g_mutex);
+  std::scoped_lock<std::mutex> lock{g_mutex};
   return g_active_clients.find({name, domain}) != g_active_clients.end();
 }
 
 void RtspClient::stop_all() {
-  std::lock_guard<std::mutex> lock(g_mutex);
+  std::scoped_lock<std::mutex> lock{g_mutex};
   auto it = g_active_clients.begin();
   while (it != g_active_clients.end()) {
     BOOST_LOG_TRIVIAL(info) << "rtsp_client:: stopping client "

daemon/rtsp_client.hpp

@@ -41,7 +41,7 @@ class RtspClient {
                                       const std::string& domain,
                                       const RtspSource& source)>;
 
-  static std::pair<bool, RtspSource> process(Observer callback,
+  static std::pair<bool, RtspSource> process(const Observer& callback,
                                              const std::string& name,
                                              const std::string& domain,
                                              const std::string& path,
@@ -57,11 +57,11 @@ class RtspClient {
       const std::string& address,
       const std::string& port = dft_port);
 
-  inline static std::atomic<uint16_t> g_seq_number{0};
   inline static std::map<
       std::pair<std::string /*name*/, std::string /*domain*/>,
       boost::asio::ip::tcp::iostream* /*stream*/>
       g_active_clients;
+  inline static std::atomic<uint16_t> g_seq_number{0};
   inline static std::mutex g_mutex;
 };
 

daemon/rtsp_server.cpp

@@ -25,7 +25,7 @@ bool RtspServer::update_source(uint8_t id,
                                const std::string& sdp) {
   bool ret = false;
   BOOST_LOG_TRIVIAL(debug) << "rtsp_server:: added source " << name;
-  std::lock_guard<std::mutex> lock(mutex_);
+  std::scoped_lock<std::mutex> lock{mutex_};
   for (unsigned int i = 0; i < sessions_.size(); i++) {
     auto session = sessions_[i].lock();
     if (session != nullptr) {
@@ -39,7 +39,7 @@ bool RtspServer::update_source(uint8_t id,
 void RtspServer::accept() {
   acceptor_.async_accept(socket_, [this](boost::system::error_code ec) {
     if (!ec) {
-      std::lock_guard<std::mutex> lock(mutex_);
+      std::scoped_lock<std::mutex> lock{mutex_};
       /* check for free sessions */
       unsigned int i = 0;
       for (; i < sessions_.size(); i++) {
@@ -73,8 +73,8 @@ bool RtspSession::announce(uint8_t id,
    * and the specified source id has been described on this session send update
    */
   if (cseq_ < 0 && source_ids_.find(id) != source_ids_.end()) {
-    std::string path(std::string("/by-name/") +
+    std::string path(std::string("/by-name/") + config_->get_node_id() + " " +
-                     config_->get_node_id() + " " + name);
+                     name);
     std::stringstream ss;
     ss << "ANNOUNCE rtsp://" << address << ":" << std::to_string(port)
        << httplib::detail::encode_url(path) << " RTSP/1.0\r\n"
@@ -170,17 +170,18 @@ void RtspSession::build_response(const std::string& url) {
     send_error(400, "Bad Request");
     return;
   }
-  auto path = std::get<4>(res);
+  const auto& path = std::get<4>(res);
-  auto base_path =
+  auto base_path = std::string("/by-name/") + config_->get_node_id() + " ";
-      std::string("/by-name/") + config_->get_node_id() + " ";
   uint8_t id = SessionManager::stream_id_max + 1;
   if (path.rfind(base_path) != std::string::npos) {
     /* extract the source name from path and retrive the id */
     id = session_manager_->get_source_id(path.substr(base_path.length()));
   } else if (path.rfind("/by-id/") != std::string::npos) {
     try {
-      id = stoi(path.substr(7));
+      id = (uint8_t)stoi(path.substr(7));
     } catch (...) {
+      id = SessionManager::stream_id_max + 1;
+      ;
     }
   }
   if (id != (SessionManager::stream_id_max + 1)) {

daemon/rtsp_server.hpp

@@ -44,10 +44,7 @@ class RtspSession : public std::enable_shared_from_this<RtspSession> {
               tcp::socket socket)
       : config_(config),
         session_manager_(session_manager),
-        socket_(std::move(socket)),
+        socket_(std::move(socket)) {}
-        length_{0},
-        cseq_{-1},
-        consumed_{0} {}
 
   virtual ~RtspSession() {
     BOOST_LOG_TRIVIAL(debug) << "rtsp_server:: session end";
@@ -87,18 +84,14 @@ class RtspServer {
                                            2};
 
   RtspServer() = delete;
-  RtspServer(std::shared_ptr<SessionManager> session_manager,
+  explicit RtspServer(std::shared_ptr<SessionManager> session_manager,
                       std::shared_ptr<Config> config)
       : session_manager_(session_manager),
         config_(config),
-        sessions_(session_num_max),
-        sessions_start_point_(session_num_max),
         acceptor_(io_service_,
                   tcp::endpoint(boost::asio::ip::address::from_string(
                                     config_->get_ip_addr_str()),
-                                config_->get_rtsp_port())),
+                                config_->get_rtsp_port())) {}
-        socket_(io_service_) {}
-
   bool init() {
     accept();
     /* start rtsp server on a separate thread */
@@ -135,8 +128,8 @@ class RtspServer {
   boost::asio::io_service io_service_;
   std::shared_ptr<SessionManager> session_manager_;
   std::shared_ptr<Config> config_;
-  std::vector<std::weak_ptr<RtspSession> > sessions_;
+  std::vector<std::weak_ptr<RtspSession> > sessions_{session_num_max};
-  std::vector<time_point<steady_clock> > sessions_start_point_;
+  std::vector<time_point<steady_clock> > sessions_start_point_{session_num_max};
   tcp::acceptor acceptor_;
   tcp::socket socket_{io_service_};
   std::future<void> res_;

daemon/sap.cpp

@@ -106,7 +106,7 @@ bool SAP::receive(bool& is_announce,
     is_announce = (buffer[0] == 0x20);
     memcpy(&msg_id_hash, buffer + 2, sizeof(msg_id_hash));
     memcpy(&addr, buffer + 4, sizeof(addr));
-    for (int i = 8; buffer[i] != 0 && i < static_cast<int>(length); i++) {
+    for (int i = 8; i < static_cast<int>(length) && buffer[i] != 0; i++) {
       buffer[i] = std::tolower(buffer[i]);
     }
     if (!memcmp(buffer + 8, "application/sdp", 16)) {

daemon/sap.hpp

@@ -37,7 +37,7 @@ class SAP {
   constexpr static uint16_t max_length = 4096;
 
   SAP() = delete;
-  SAP(const std::string& sap_mcast_addr);
+  explicit SAP(const std::string& sap_mcast_addr);
 
   bool set_multicast_interface(const std::string& interface_ip);
   bool announcement(uint16_t msg_id_hash,

daemon/session_manager.cpp

@@ -409,7 +409,7 @@ bool SessionManager::load_status() {
   return true;
 }
 
-bool SessionManager::save_status() {
+bool SessionManager::save_status() const {
   if (config_->get_status_file().empty()) {
     return true;
   }
@@ -443,7 +443,8 @@ uint8_t SessionManager::get_source_id(const std::string& name) const {
   return it != source_names_.end() ? it->second : (stream_id_max + 1);
 }
 
-void SessionManager::add_source_observer(ObserverType type, Observer cb) {
+void SessionManager::add_source_observer(ObserverType type,
+                                         const Observer& cb) {
   switch (type) {
     case ObserverType::add_source:
       add_source_observers.push_back(cb);
@@ -459,7 +460,7 @@ void SessionManager::add_source_observer(ObserverType type, Observer cb) {
 
 void SessionManager::on_add_source(const StreamSource& source,
                                    const StreamInfo& info) {
-  for (auto cb : add_source_observers) {
+  for (const auto& cb : add_source_observers) {
     cb(source.id, source.name, get_source_sdp_(source.id, info));
   }
   if (IN_MULTICAST(info.stream.m_ui32DestIP)) {
@@ -470,8 +471,8 @@ void SessionManager::on_add_source(const StreamSource& source,
 }
 
 void SessionManager::on_remove_source(const StreamInfo& info) {
-  for (auto cb : remove_source_observers) {
+  for (const auto& cb : remove_source_observers) {
-    cb(info.stream.m_uiId, info.stream.m_cName, {});
+    cb((uint8_t)info.stream.m_uiId, info.stream.m_cName, {});
   }
   if (IN_MULTICAST(info.stream.m_ui32DestIP)) {
     igmp_.leave(config_->get_ip_addr_str(),
@@ -531,13 +532,14 @@ std::error_code SessionManager::add_source(const StreamSource& source) {
         config_->get_interface_name(),
         ip::address_v4(info.stream.m_ui32DestIP).to_string());
     int retry = 3;
-    while (!mac_addr.second.length() && retry--) {
+    while (!mac_addr.second.length() && retry > 0) {
       // if not in cache already try to populate the MAC cache
       (void)echo_try_connect(
           ip::address_v4(info.stream.m_ui32DestIP).to_string());
       mac_addr = get_mac_from_arp_cache(
           config_->get_interface_name(),
           ip::address_v4(info.stream.m_ui32DestIP).to_string());
+      retry--;
     }
     if (!mac_addr.second.length()) {
       BOOST_LOG_TRIVIAL(error)
@@ -693,8 +695,7 @@ std::error_code SessionManager::remove_source(uint32_t id) {
   }
 
   std::error_code ret;
-  const auto& info = (*it).second;
+  if (const auto& info = (*it).second; info.enabled) {
-  if (info.enabled) {
     ret = driver_->remove_rtp_stream(info.handle);
     if (!ret) {
       on_remove_source(info);
@@ -808,7 +809,7 @@ std::error_code SessionManager::add_sink(const StreamSink& sink) {
       return DaemonErrc::cannot_parse_sdp;
     }
 
-    info.sink_sdp = std::move(sink.sdp);
+    info.sink_sdp = sink.sdp;
   }
   info.sink_source = sink.source;
   info.sink_use_sdp = true;  // save back and use with SDP file
@@ -1076,7 +1077,7 @@ void SessionManager::on_update_sources() {
   // trigger sources SDP file update
   sources_mutex_.lock();
   for (auto& [id, info] : sources_) {
-    for (auto cb : update_source_observers) {
+    for (const auto& cb : update_source_observers) {
       info.session_version++;
       cb(id, info.stream.m_cName, get_source_sdp_(id, info));
     }
@@ -1102,9 +1103,8 @@ void SessionManager::on_ptp_status_changed(const std::string& status) const {
       for (int i = STDERR_FILENO + 1; i < fdlimit; i++)
         close(i);
 
-      char* argv_list[] = {
+      char* argv_list[] = {(char*)(config_->get_ptp_status_script().c_str()),
-          const_cast<char*>(config_->get_ptp_status_script().c_str()),
+                           (char*)(status.c_str()), nullptr};
-          const_cast<char*>(status.c_str()), NULL};
 
       execv(config_->get_ptp_status_script().c_str(), argv_list);
       exit(0);

daemon/session_manager.hpp

@@ -86,7 +86,7 @@ struct PTPStatus {
 struct StreamInfo {
   TRTP_stream_info stream;
   uint64_t handle{0};
-  bool enabled{0};
+  bool enabled{false};
   bool refclk_ptp_traceable{false};
   bool ignore_refclk_gmid{false};
   std::string io;
@@ -109,7 +109,7 @@ class SessionManager {
   SessionManager() = delete;
   SessionManager(const SessionManager&) = delete;
   SessionManager& operator=(const SessionManager&) = delete;
-  virtual ~SessionManager() { terminate(); };
+  virtual ~SessionManager() = default;
 
   // session manager interface
   bool init() {
@@ -127,10 +127,10 @@ class SessionManager {
     if (running_) {
       running_ = false;
       auto ret = res_.get();
-      for (auto source : get_sources()) {
+      for (const auto& source : get_sources()) {
         remove_source(source.id);
       }
-      for (auto sink : get_sinks()) {
+      for (const auto& sink : get_sinks()) {
         remove_sink(sink.id);
       }
       return ret;
@@ -148,7 +148,7 @@ class SessionManager {
   enum class ObserverType { add_source, remove_source, update_source };
   using Observer = std::function<
       bool(uint8_t id, const std::string& name, const std::string& sdp)>;
-  void add_source_observer(ObserverType type, Observer cb);
+  void add_source_observer(ObserverType type, const Observer& cb);
 
   std::error_code add_sink(const StreamSink& sink);
   std::error_code get_sink(uint8_t id, StreamSink& sink) const;
@@ -164,7 +164,7 @@ class SessionManager {
   void get_ptp_status(PTPStatus& status) const;
 
   bool load_status();
-  bool save_status();
+  bool save_status() const;
 
   size_t process_sap();
 
@@ -200,7 +200,7 @@ class SessionManager {
   bool parse_sdp(const std::string sdp, StreamInfo& info) const;
   bool worker();
   // singleton, use create() to build
-  SessionManager(std::shared_ptr<DriverManager> driver,
+  explicit SessionManager(std::shared_ptr<DriverManager> driver,
                           std::shared_ptr<Browser> browser,
                           std::shared_ptr<Config> config)
       : browser_(browser), driver_(driver), config_(config) {
@@ -208,6 +208,7 @@ class SessionManager {
     ptp_config_.dscp = config->get_ptp_dscp();
   };
 
+ private:
   std::shared_ptr<Browser> browser_;
   std::shared_ptr<DriverManager> driver_;
   std::shared_ptr<Config> config_;

daemon/tests/daemon_test.cpp

@@ -70,7 +70,8 @@ struct DaemonInstance {
     auto pid = daemon_.native_handle();
     /* trigger normal daemon termination */
     kill(pid, SIGTERM);
-    daemon_.wait();
+    std::error_code ec;
+    daemon_.wait(ec);
     BOOST_REQUIRE_MESSAGE(!daemon_.exit_code(), "daemon exited normally");
     ok = false;
   }