From 5dab23e0171fd9fcb1976ac13e484ee7cf166e29 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 Jul 2020 18:53:25 -0400 Subject: [PATCH 1/2] nvflinger: Use return value of Lock() comex reported in #4424 that we were incorrectly discarding the return value of Lock() which is correct. --- src/core/hle/service/nvflinger/nvflinger.cpp | 2 +- src/core/hle/service/nvflinger/nvflinger.h | 2 +- src/core/hle/service/vi/vi.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/hle/service/nvflinger/nvflinger.cpp b/src/core/hle/service/nvflinger/nvflinger.cpp index 789856118..e561bf654 100644 --- a/src/core/hle/service/nvflinger/nvflinger.cpp +++ b/src/core/hle/service/nvflinger/nvflinger.cpp @@ -68,7 +68,7 @@ NVFlinger::NVFlinger(Core::System& system) : system(system) { // Schedule the screen composition events composition_event = Core::Timing::CreateEvent( "ScreenComposition", [this](u64, std::chrono::nanoseconds ns_late) { - Lock(); + const auto guard = Lock(); Compose(); const auto ticks = std::chrono::nanoseconds{GetNextTicks()}; diff --git a/src/core/hle/service/nvflinger/nvflinger.h b/src/core/hle/service/nvflinger/nvflinger.h index e4959a9af..0be55e442 100644 --- a/src/core/hle/service/nvflinger/nvflinger.h +++ b/src/core/hle/service/nvflinger/nvflinger.h @@ -86,7 +86,7 @@ public: s64 GetNextTicks() const; - std::unique_lock Lock() { + std::unique_lock Lock() const { return std::unique_lock{*guard}; } diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index ea7b4ae13..825d11a3f 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -511,7 +511,7 @@ private: LOG_DEBUG(Service_VI, "called. id=0x{:08X} transaction={:X}, flags=0x{:08X}", id, static_cast(transaction), flags); - nv_flinger->Lock(); + const auto guard = nv_flinger->Lock(); auto& buffer_queue = nv_flinger->FindBufferQueue(id); switch (transaction) { @@ -551,7 +551,7 @@ private: [=](std::shared_ptr thread, Kernel::HLERequestContext& ctx, Kernel::ThreadWakeupReason reason) { // Repeat TransactParcel DequeueBuffer when a buffer is available - nv_flinger->Lock(); + const auto guard = nv_flinger->Lock(); auto& buffer_queue = nv_flinger->FindBufferQueue(id); auto result = buffer_queue.DequeueBuffer(width, height); ASSERT_MSG(result != std::nullopt, "Could not dequeue buffer."); From 7b070bbf62bc42daba7e28b292413ddc2ecf450a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sat, 25 Jul 2020 18:55:53 -0400 Subject: [PATCH 2/2] nvflinger: Mark interface functions with return values as [[nodiscard]] Not using the return value of these functions are undeniably the source of a bug. This way we allow compilers to loudly make any future misuses evident. --- src/core/hle/service/nvflinger/nvflinger.h | 30 ++++++++++------------ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/core/hle/service/nvflinger/nvflinger.h b/src/core/hle/service/nvflinger/nvflinger.h index 0be55e442..ff85cbba6 100644 --- a/src/core/hle/service/nvflinger/nvflinger.h +++ b/src/core/hle/service/nvflinger/nvflinger.h @@ -54,12 +54,12 @@ public: /// Opens the specified display and returns the ID. /// /// If an invalid display name is provided, then an empty optional is returned. - std::optional OpenDisplay(std::string_view name); + [[nodiscard]] std::optional OpenDisplay(std::string_view name); /// Creates a layer on the specified display and returns the layer ID. /// /// If an invalid display ID is specified, then an empty optional is returned. - std::optional CreateLayer(u64 display_id); + [[nodiscard]] std::optional CreateLayer(u64 display_id); /// Closes a layer on all displays for the given layer ID. void CloseLayer(u64 layer_id); @@ -67,41 +67,39 @@ public: /// Finds the buffer queue ID of the specified layer in the specified display. /// /// If an invalid display ID or layer ID is provided, then an empty optional is returned. - std::optional FindBufferQueueId(u64 display_id, u64 layer_id) const; + [[nodiscard]] std::optional FindBufferQueueId(u64 display_id, u64 layer_id) const; /// Gets the vsync event for the specified display. /// /// If an invalid display ID is provided, then nullptr is returned. - std::shared_ptr FindVsyncEvent(u64 display_id) const; + [[nodiscard]] std::shared_ptr FindVsyncEvent(u64 display_id) const; /// Obtains a buffer queue identified by the ID. - BufferQueue& FindBufferQueue(u32 id); + [[nodiscard]] BufferQueue& FindBufferQueue(u32 id); /// Obtains a buffer queue identified by the ID. - const BufferQueue& FindBufferQueue(u32 id) const; + [[nodiscard]] const BufferQueue& FindBufferQueue(u32 id) const; /// Performs a composition request to the emulated nvidia GPU and triggers the vsync events when /// finished. void Compose(); - s64 GetNextTicks() const; + [[nodiscard]] s64 GetNextTicks() const; - std::unique_lock Lock() const { - return std::unique_lock{*guard}; - } + [[nodiscard]] std::unique_lock Lock() const { return std::unique_lock{*guard}; } -private: - /// Finds the display identified by the specified ID. - VI::Display* FindDisplay(u64 display_id); + private : + /// Finds the display identified by the specified ID. + [[nodiscard]] VI::Display* FindDisplay(u64 display_id); /// Finds the display identified by the specified ID. - const VI::Display* FindDisplay(u64 display_id) const; + [[nodiscard]] const VI::Display* FindDisplay(u64 display_id) const; /// Finds the layer identified by the specified ID in the desired display. - VI::Layer* FindLayer(u64 display_id, u64 layer_id); + [[nodiscard]] VI::Layer* FindLayer(u64 display_id, u64 layer_id); /// Finds the layer identified by the specified ID in the desired display. - const VI::Layer* FindLayer(u64 display_id, u64 layer_id) const; + [[nodiscard]] const VI::Layer* FindLayer(u64 display_id, u64 layer_id) const; static void VSyncThread(NVFlinger& nv_flinger);