diff options
author | mark a. foltz <mfoltz@chromium.org> | 2020-11-06 17:34:15 -0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-11-09 18:02:46 +0000 |
commit | 1030c88a4bb869ab91ca5abf28471e50183f1bcb (patch) | |
tree | 354d9b3201d1778d0d1415a4f95b4861544916a3 /cast/sender | |
parent | 9abc1e30cfe55eab9360fe0a03bb06329f589b67 (diff) | |
download | openscreen-1030c88a4bb869ab91ca5abf28471e50183f1bcb.tar.gz |
[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>
Diffstat (limited to 'cast/sender')
-rw-r--r-- | cast/sender/cast_app_discovery_service_impl.cc | 15 | ||||
-rw-r--r-- | cast/sender/cast_app_discovery_service_impl.h | 5 | ||||
-rw-r--r-- | cast/sender/public/cast_app_discovery_service.cc | 14 | ||||
-rw-r--r-- | cast/sender/public/cast_app_discovery_service.h | 15 |
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; }; |