From d0f6131ba40e8336eb4f62f19f0becc5b4e6cd20 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Sep 2017 14:45:53 +0200 Subject: [PATCH] output/Interface: allow Pause() to throw exception Coverity discovered that the Pulse plugin could throw exceptions from Pause(), but that method was marked "noexcept" because its caller was not designed to catch exceptions. So instead of avoiding exceptions (by catching and logging them in each and every implementation), let's allow them, and do the catch/log game in the MPD core. --- src/output/Interface.hxx | 5 ++++- src/output/plugins/JackOutputPlugin.cxx | 4 ++-- src/output/plugins/PulseOutputPlugin.cxx | 4 ++-- src/output/plugins/ShoutOutputPlugin.cxx | 12 ++++-------- src/output/plugins/httpd/HttpdInternal.hxx | 2 +- src/output/plugins/httpd/HttpdOutputPlugin.cxx | 2 +- src/output/plugins/sles/SlesOutputPlugin.cxx | 10 ++++------ 7 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/output/Interface.hxx b/src/output/Interface.hxx index 70e47b273..ddc6ffa4b 100644 --- a/src/output/Interface.hxx +++ b/src/output/Interface.hxx @@ -140,8 +140,11 @@ public: * * @return false on error (output will be closed by caller), * true for continue to pause + * + * Instead of returning false, the method may throw an + * exception, which will be logged. */ - virtual bool Pause() noexcept { + virtual bool Pause() { /* fail because this method is not implemented */ return false; } diff --git a/src/output/plugins/JackOutputPlugin.cxx b/src/output/plugins/JackOutputPlugin.cxx index a1778888d..70a588610 100644 --- a/src/output/plugins/JackOutputPlugin.cxx +++ b/src/output/plugins/JackOutputPlugin.cxx @@ -135,7 +135,7 @@ struct JackOutput final : AudioOutput { size_t Play(const void *chunk, size_t size) override; - bool Pause() noexcept override; + bool Pause() override; }; static constexpr Domain jack_output_domain("jack_output"); @@ -661,7 +661,7 @@ JackOutput::Play(const void *chunk, size_t size) } inline bool -JackOutput::Pause() noexcept +JackOutput::Pause() { if (shutdown) return false; diff --git a/src/output/plugins/PulseOutputPlugin.cxx b/src/output/plugins/PulseOutputPlugin.cxx index fa603d5f7..11fd960e9 100644 --- a/src/output/plugins/PulseOutputPlugin.cxx +++ b/src/output/plugins/PulseOutputPlugin.cxx @@ -103,7 +103,7 @@ public: std::chrono::steady_clock::duration Delay() const noexcept override; size_t Play(const void *chunk, size_t size) override; void Cancel() noexcept override; - bool Pause() noexcept override; + bool Pause() override; private: /** @@ -826,7 +826,7 @@ PulseOutput::Cancel() noexcept } bool -PulseOutput::Pause() noexcept +PulseOutput::Pause() { assert(mainloop != nullptr); assert(stream != nullptr); diff --git a/src/output/plugins/ShoutOutputPlugin.cxx b/src/output/plugins/ShoutOutputPlugin.cxx index b2aa904b4..70c9fc284 100644 --- a/src/output/plugins/ShoutOutputPlugin.cxx +++ b/src/output/plugins/ShoutOutputPlugin.cxx @@ -65,7 +65,7 @@ struct ShoutOutput final : AudioOutput { void SendTag(const Tag &tag) override; size_t Play(const void *chunk, size_t size) override; void Cancel() noexcept override; - bool Pause() noexcept override; + bool Pause() override; private: void WritePage(); @@ -378,16 +378,12 @@ ShoutOutput::Play(const void *chunk, size_t size) } bool -ShoutOutput::Pause() noexcept +ShoutOutput::Pause() { static char silence[1020]; - try { - encoder->Write(silence, sizeof(silence)); - WritePage(); - } catch (const std::runtime_error &) { - return false; - } + encoder->Write(silence, sizeof(silence)); + WritePage(); return true; } diff --git a/src/output/plugins/httpd/HttpdInternal.hxx b/src/output/plugins/httpd/HttpdInternal.hxx index f7df99c57..5e6a39dbe 100644 --- a/src/output/plugins/httpd/HttpdInternal.hxx +++ b/src/output/plugins/httpd/HttpdInternal.hxx @@ -252,7 +252,7 @@ public: void CancelAllClients(); void Cancel() noexcept override; - bool Pause() noexcept override; + bool Pause() override; private: virtual void RunDeferred() override; diff --git a/src/output/plugins/httpd/HttpdOutputPlugin.cxx b/src/output/plugins/httpd/HttpdOutputPlugin.cxx index bfffb6354..95e3edaab 100644 --- a/src/output/plugins/httpd/HttpdOutputPlugin.cxx +++ b/src/output/plugins/httpd/HttpdOutputPlugin.cxx @@ -371,7 +371,7 @@ HttpdOutput::Play(const void *chunk, size_t size) } bool -HttpdOutput::Pause() noexcept +HttpdOutput::Pause() { pause = true; diff --git a/src/output/plugins/sles/SlesOutputPlugin.cxx b/src/output/plugins/sles/SlesOutputPlugin.cxx index d1ba818f7..9ae6ea584 100644 --- a/src/output/plugins/sles/SlesOutputPlugin.cxx +++ b/src/output/plugins/sles/SlesOutputPlugin.cxx @@ -102,7 +102,7 @@ private: void Drain() override; void Cancel() noexcept override; - bool Pause() noexcept override; + bool Pause() override; private: void PlayedCallback(); @@ -368,7 +368,7 @@ SlesOutput::Cancel() noexcept } bool -SlesOutput::Pause() noexcept +SlesOutput::Pause() { cancel = false; @@ -378,10 +378,8 @@ SlesOutput::Pause() noexcept pause = true; SLresult result = play.SetPlayState(SL_PLAYSTATE_PAUSED); - if (result != SL_RESULT_SUCCESS) { - FormatError(sles_domain, "Play.SetPlayState(PAUSED) failed"); - return false; - } + if (result != SL_RESULT_SUCCESS) + throw std::runtime_error("Play.SetPlayState(PAUSED) failed"); return true; }