[Open Screen] Fix two unitialized reads dicovered by MSAN.

- Fix read of unitialized watch duration in OSP
- Fix uninitialized endpoint_id in OSP

Also, update Subscription to follow rule of 3/5, and simplify how
query IDs are allocated in the cast_app_discovery_service.

Bug: chromium:1143946
Change-Id: I78f3dd630626eaf47930d8a5c9bfdc3d05b49fb6
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2511074
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Brandon Tolsch <btolsch@chromium.org>

4 files changed, 16 insertions, 33 deletions

diff --git a/cast/sender/cast_app_discovery_service_impl.cc b/cast/sender/cast_app_discovery_service_impl.cc
index 955ab0e4..fd071ca2 100644
--- a/cast/sender/cast_app_discovery_service_impl.cc
+++ b/cast/sender/cast_app_discovery_service_impl.cc
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <chrono>
+#include <utility>
 
 #include "cast/sender/public/cast_media_source.h"
 #include "util/osp_logging.h"
@@ -47,7 +48,7 @@ CastAppDiscoveryServiceImpl::StartObservingAvailability(
   }
 
   auto& callbacks = avail_queries_[source_id];
-  uint32_t query_id = GetNextAvailabilityQueryId();
+  uint32_t query_id = next_avail_query_id_++;
   callbacks.push_back({query_id, std::move(callback)});
   if (callbacks.size() == 1) {
     // NOTE: Even though we retain availability results for an app unregistered
@@ -62,7 +63,7 @@ CastAppDiscoveryServiceImpl::StartObservingAvailability(
     }
   }
 
-  return MakeSubscription(this, query_id);
+  return Subscription(this, query_id);
 }
 
 void CastAppDiscoveryServiceImpl::Refresh() {
@@ -176,16 +177,6 @@ bool CastAppDiscoveryServiceImpl::ShouldRefreshAppAvailability(
   return false;
 }
 
-uint32_t CastAppDiscoveryServiceImpl::GetNextAvailabilityQueryId() {
-  if (free_query_ids_.empty()) {
-    return next_avail_query_id_++;
-  } else {
-    uint32_t id = free_query_ids_.back();
-    free_query_ids_.pop_back();
-    return id;
-  }
-}
-
 void CastAppDiscoveryServiceImpl::RemoveAvailabilityCallback(uint32_t id) {
   for (auto entry = avail_queries_.begin(); entry != avail_queries_.end();
        ++entry) {
diff --git a/cast/sender/cast_app_discovery_service_impl.h b/cast/sender/cast_app_discovery_service_impl.h
index 4093311a..fa577808 100644
--- a/cast/sender/cast_app_discovery_service_impl.h
+++ b/cast/sender/cast_app_discovery_service_impl.h
@@ -70,8 +70,6 @@ class CastAppDiscoveryServiceImpl : public CastAppDiscoveryService {
                                     const std::string& app_id,
                                     Clock::time_point now) const;
 
-  uint32_t GetNextAvailabilityQueryId();
-
   void RemoveAvailabilityCallback(uint32_t id) override;
 
   std::map<std::string, ServiceInfo> receivers_by_id_;
@@ -81,8 +79,7 @@ class CastAppDiscoveryServiceImpl : public CastAppDiscoveryService {
   std::map<std::string, std::vector<AvailabilityCallbackEntry>> avail_queries_;
 
   // Callback ID tracking.
-  uint32_t next_avail_query_id_;
-  std::vector<uint32_t> free_query_ids_;
+  uint32_t next_avail_query_id_ = 1U;
 
   CastPlatformClient* const platform_client_;
 
diff --git a/cast/sender/public/cast_app_discovery_service.cc b/cast/sender/public/cast_app_discovery_service.cc
index c561b386..8299aff4 100644
--- a/cast/sender/public/cast_app_discovery_service.cc
+++ b/cast/sender/public/cast_app_discovery_service.cc
@@ -17,14 +17,16 @@ CastAppDiscoveryService::Subscription::Subscription(Subscription&& other)
   other.discovery_service_ = nullptr;
 }
 
-CastAppDiscoveryService::Subscription::~Subscription() {
-  Reset();
+CastAppDiscoveryService::Subscription&
+CastAppDiscoveryService::Subscription::operator=(Subscription&& other) {
+  id_ = other.id_;
+  discovery_service_ = other.discovery_service_;
+  other.discovery_service_ = nullptr;
+  return *this;
 }
 
-CastAppDiscoveryService::Subscription& CastAppDiscoveryService::Subscription::
-operator=(Subscription other) {
-  Swap(other);
-  return *this;
+CastAppDiscoveryService::Subscription::~Subscription() {
+  Reset();
 }
 
 void CastAppDiscoveryService::Subscription::Reset() {
diff --git a/cast/sender/public/cast_app_discovery_service.h b/cast/sender/public/cast_app_discovery_service.h
index ccb586ff..6ce5ee78 100644
--- a/cast/sender/public/cast_app_discovery_service.h
+++ b/cast/sender/public/cast_app_discovery_service.h
@@ -23,17 +23,18 @@ class CastAppDiscoveryService {
 
   class Subscription {
    public:
+    Subscription(CastAppDiscoveryService* discovery_service, uint32_t id);
     Subscription(Subscription&&);
+    Subscription(Subscription&) = delete;
+    Subscription& operator=(Subscription&&);
+    Subscription& operator=(const Subscription&) = delete;
     ~Subscription();
-    Subscription& operator=(Subscription);
 
     void Reset();
 
    private:
     friend class CastAppDiscoveryService;
 
-    Subscription(CastAppDiscoveryService* discovery_service, uint32_t id);
-
     void Swap(Subscription& other);
 
     CastAppDiscoveryService* discovery_service_;
@@ -57,15 +58,7 @@ class CastAppDiscoveryService {
   // this method when the user initiates a user gesture.
   virtual void Refresh() = 0;
 
- protected:
-  Subscription MakeSubscription(CastAppDiscoveryService* discovery_service,
-                                uint32_t id) {
-    return Subscription(discovery_service, id);
-  }
-
  private:
-  friend class Subscription;
-
   virtual void RemoveAvailabilityCallback(uint32_t id) = 0;
 };