player/Control: wrap DetachedSong* in std::unique_ptr

src/Partition.cxx

@@ -88,11 +88,9 @@ Partition::DatabaseModified(const Database &db)
 void
 Partition::TagModified()
 {
-	DetachedSong *song = pc.LockReadTaggedSong();
+	auto song = pc.LockReadTaggedSong();
-	if (song != nullptr) {
+	if (song)
 		playlist.TagModified(std::move(*song));
-		delete song;
-	}
 }
 
 void

src/player/Control.cxx

@@ -44,8 +44,6 @@ PlayerControl::PlayerControl(PlayerListener &_listener,
 
 PlayerControl::~PlayerControl() noexcept
 {
-	delete next_song;
-	delete tagged_song;
 }
 
 bool
@@ -61,12 +59,12 @@ PlayerControl::WaitOutputConsumed(unsigned threshold) noexcept
 }
 
 void
-PlayerControl::Play(DetachedSong *song)
+PlayerControl::Play(std::unique_ptr<DetachedSong> song)
 {
 	assert(song != nullptr);
 
 	const std::lock_guard<Mutex> protect(mutex);
-	SeekLocked(song, SongTime::zero());
+	SeekLocked(std::move(song), SongTime::zero());
 
 	if (state == PlayerState::PAUSE)
 		/* if the player was paused previously, we need to
@@ -192,28 +190,51 @@ void
 PlayerControl::LockSetTaggedSong(const DetachedSong &song) noexcept
 {
 	const std::lock_guard<Mutex> protect(mutex);
-	delete tagged_song;
+	tagged_song.reset();
-	tagged_song = new DetachedSong(song);
+	tagged_song = std::make_unique<DetachedSong>(song);
 }
 
 void
 PlayerControl::ClearTaggedSong() noexcept
 {
-	delete tagged_song;
+	tagged_song.reset();
-	tagged_song = nullptr;
+}
+
+std::unique_ptr<DetachedSong>
+PlayerControl::ReadTaggedSong() noexcept
+{
+	return std::exchange(tagged_song, nullptr);
+}
+
+std::unique_ptr<DetachedSong>
+PlayerControl::LockReadTaggedSong() noexcept
+{
+	const std::lock_guard<Mutex> protect(mutex);
+	return ReadTaggedSong();
 }
 
 void
-PlayerControl::LockEnqueueSong(DetachedSong *song) noexcept
+PlayerControl::LockEnqueueSong(std::unique_ptr<DetachedSong> song) noexcept
 {
 	assert(song != nullptr);
 
 	const std::lock_guard<Mutex> protect(mutex);
-	EnqueueSongLocked(song);
+	EnqueueSongLocked(std::move(song));
 }
 
 void
-PlayerControl::SeekLocked(DetachedSong *song, SongTime t)
+PlayerControl::EnqueueSongLocked(std::unique_ptr<DetachedSong> song) noexcept
+{
+	assert(song != nullptr);
+	assert(next_song == nullptr);
+
+	next_song = std::move(song);
+	seek_time = SongTime::zero();
+	SynchronousCommand(PlayerCommand::QUEUE);
+}
+
+void
+PlayerControl::SeekLocked(std::unique_ptr<DetachedSong> song, SongTime t)
 {
 	assert(song != nullptr);
 
@@ -227,7 +248,7 @@ PlayerControl::SeekLocked(DetachedSong *song, SongTime t)
 	assert(next_song == nullptr);
 
 	ClearError();
-	next_song = song;
+	next_song = std::move(song);
 	seek_time = t;
 	SynchronousCommand(PlayerCommand::SEEK);
 
@@ -242,13 +263,13 @@ PlayerControl::SeekLocked(DetachedSong *song, SongTime t)
 }
 
 void
-PlayerControl::LockSeek(DetachedSong *song, SongTime t)
+PlayerControl::LockSeek(std::unique_ptr<DetachedSong> song, SongTime t)
 {
 	assert(song != nullptr);
 
 	{
 		const std::lock_guard<Mutex> protect(mutex);
-		SeekLocked(song, t);
+		SeekLocked(std::move(song), t);
 	}
 
 	idle_add(IDLE_PLAYER);

src/player/Control.hxx

@@ -31,6 +31,7 @@
 #include "ReplayGainMode.hxx"
 
 #include <exception>
+#include <memory>
 
 #include <stdint.h>
 
@@ -156,7 +157,7 @@ struct PlayerControl final : AudioOutputClient {
 	 * Protected by #mutex.  Set by the PlayerThread and consumed
 	 * by the main thread.
 	 */
-	DetachedSong *tagged_song = nullptr;
+	std::unique_ptr<DetachedSong> tagged_song;
 
 	uint16_t bit_rate;
 	AudioFormat audio_format;
@@ -169,7 +170,7 @@ struct PlayerControl final : AudioOutputClient {
 	 * This is a duplicate, and must be freed when this attribute
 	 * is cleared.
 	 */
-	DetachedSong *next_song = nullptr;
+	std::unique_ptr<DetachedSong> next_song;
 
 	SongTime seek_time;
 
@@ -341,10 +342,9 @@ public:
 	/**
 	 * Throws std::runtime_error or #Error on error.
 	 *
-	 * @param song the song to be queued; the given instance will
+	 * @param song the song to be queued
-	 * be owned and freed by the player
 	 */
-	void Play(DetachedSong *song);
+	void Play(std::unique_ptr<DetachedSong> song);
 
 	/**
 	 * see PlayerCommand::CANCEL
@@ -452,45 +452,31 @@ public:
 	 *
 	 * Caller must lock the object.
 	 */
-	DetachedSong *ReadTaggedSong() noexcept {
+	std::unique_ptr<DetachedSong> ReadTaggedSong() noexcept;
-		DetachedSong *result = tagged_song;
-		tagged_song = nullptr;
-		return result;
-	}
 
 	/**
 	 * Like ReadTaggedSong(), but locks and unlocks the object.
 	 */
-	DetachedSong *LockReadTaggedSong() noexcept {
+	std::unique_ptr<DetachedSong> LockReadTaggedSong() noexcept;
-		const std::lock_guard<Mutex> protect(mutex);
-		return ReadTaggedSong();
-	}
 
 	void LockStop() noexcept;
 
 	void LockUpdateAudio() noexcept;
 
 private:
-	void EnqueueSongLocked(DetachedSong *song) noexcept {
+	void EnqueueSongLocked(std::unique_ptr<DetachedSong> song) noexcept;
-		assert(song != nullptr);
-		assert(next_song == nullptr);
-
-		next_song = song;
-		seek_time = SongTime::zero();
-		SynchronousCommand(PlayerCommand::QUEUE);
-	}
 
 	/**
 	 * Throws std::runtime_error or #Error on error.
 	 */
-	void SeekLocked(DetachedSong *song, SongTime t);
+	void SeekLocked(std::unique_ptr<DetachedSong> song, SongTime t);
 
 public:
 	/**
 	 * @param song the song to be queued; the given instance will be owned
 	 * and freed by the player
 	 */
-	void LockEnqueueSong(DetachedSong *song) noexcept;
+	void LockEnqueueSong(std::unique_ptr<DetachedSong> song) noexcept;
 
 	/**
 	 * Makes the player thread seek the specified song to a position.
@@ -500,7 +486,7 @@ public:
 	 * @param song the song to be queued; the given instance will be owned
 	 * and freed by the player
 	 */
-	void LockSeek(DetachedSong *song, SongTime t);
+	void LockSeek(std::unique_ptr<DetachedSong> song, SongTime t);
 
 	void SetCrossFade(float cross_fade_seconds) noexcept;
 

src/player/Thread.cxx

@@ -90,7 +90,7 @@ class Player {
 	/**
 	 * the song currently being played
 	 */
-	DetachedSong *song;
+	std::unique_ptr<DetachedSong> song;
 
 	/**
 	 * Is cross-fading to the next song enabled?
@@ -413,9 +413,7 @@ Player::ActivateDecoder()
 
 		pc.ClearTaggedSong();
 
-		delete song;
+		song = std::exchange(pc.next_song, nullptr);
-		song = pc.next_song;
-		pc.next_song = nullptr;
 
 		elapsed_time = pc.seek_time;
 
@@ -605,8 +603,7 @@ Player::SeekDecoder()
 			ClearAndReplacePipe(dc.pipe);
 		}
 
-		delete pc.next_song;
+		pc.next_song.reset();
-		pc.next_song = nullptr;
 		queued = false;
 
 		/* wait for the decoder to complete initialization
@@ -724,8 +721,7 @@ Player::ProcessCommand()
 			StopDecoder();
 		}
 
-		delete pc.next_song;
+		pc.next_song.reset();
-		pc.next_song = nullptr;
 		queued = false;
 		pc.CommandFinished();
 		break;
@@ -1119,7 +1115,7 @@ Player::Run()
 
 	if (song != nullptr) {
 		FormatDefault(player_domain, "played \"%s\"", song->GetURI());
-		delete song;
+		song.reset();
 	}
 
 	pc.Lock();
@@ -1128,8 +1124,7 @@ Player::Run()
 
 	if (queued) {
 		assert(pc.next_song != nullptr);
-		delete pc.next_song;
+		pc.next_song.reset();
-		pc.next_song = nullptr;
 	}
 
 	pc.state = PlayerState::STOP;
@@ -1179,8 +1174,7 @@ PlayerControl::RunThread() noexcept
 			/* fall through */
 
 		case PlayerCommand::PAUSE:
-			delete next_song;
+			next_song.reset();
-			next_song = nullptr;
 
 			CommandFinished();
 			break;
@@ -1215,8 +1209,7 @@ PlayerControl::RunThread() noexcept
 			return;
 
 		case PlayerCommand::CANCEL:
-			delete next_song;
+			next_song.reset();
-			next_song = nullptr;
 
 			CommandFinished();
 			break;

src/queue/Playlist.cxx

@@ -56,7 +56,7 @@ playlist::QueueSongOrder(PlayerControl &pc, unsigned order)
 	FormatDebug(playlist_domain, "queue song %i:\"%s\"",
 		    queued, song.GetURI());
 
-	pc.LockEnqueueSong(new DetachedSong(song));
+	pc.LockEnqueueSong(std::make_unique<DetachedSong>(song));
 }
 
 void
@@ -163,7 +163,7 @@ playlist::PlayOrder(PlayerControl &pc, unsigned order)
 
 	current = order;
 
-	pc.Play(new DetachedSong(song));
+	pc.Play(std::make_unique<DetachedSong>(song));
 
 	SongStarted();
 }

src/queue/PlaylistControl.cxx

@@ -234,7 +234,7 @@ playlist::SeekSongOrder(PlayerControl &pc, unsigned i, SongTime seek_time)
 	queued = -1;
 
 	try {
-		pc.LockSeek(new DetachedSong(queue.GetOrder(i)), seek_time);
+		pc.LockSeek(std::make_unique<DetachedSong>(queue.GetOrder(i)), seek_time);
 	} catch (...) {
 		UpdateQueuedSong(pc, queued_song);
 		throw;