From d4d06da2f8ed7364ae897bb9527923c704c0892e Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Mon, 15 Feb 2021 17:34:03 +0100
Subject: [PATCH] db/simple: fix dangling LightSong::tag reference in moved
ExportedSong
After commit 1afa33c3c766af2, an old bug was revealed:
SimpleDatabase::GetSong() constructs an ExportedSong instance by
moving the return value of Song::Export(), which causes the
LightSong::tag field to be dangling on the moved-from
ExportedSong::tag_buffer. This broke tags from CUE sheets.
Closes https://github.com/MusicPlayerDaemon/MPD/issues/1070
---
NEWS | 2 ++
src/db/plugins/simple/ExportedSong.hxx | 9 +++++++++
src/song/LightSong.hxx | 13 +++++++++++++
3 files changed, 24 insertions(+)
diff --git a/NEWS b/NEWS
index 7b0615128..80ae7c3c7 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,6 @@
ver 0.22.5 (not yet released)
+* database
+ - simple: fix missing CUE sheet metadata in "addid" command
* tags
- id: translate TPE3 to Conductor, not Performer
* archive
diff --git a/src/db/plugins/simple/ExportedSong.hxx b/src/db/plugins/simple/ExportedSong.hxx
index 886be078b..37e4109b5 100644
--- a/src/db/plugins/simple/ExportedSong.hxx
+++ b/src/db/plugins/simple/ExportedSong.hxx
@@ -37,6 +37,15 @@ public:
ExportedSong(const char *_uri, Tag &&_tag) noexcept
:LightSong(_uri, tag_buffer),
tag_buffer(std::move(_tag)) {}
+
+ /* this custom move constructor is necessary so LightSong::tag
+ points to this instance's #Tag field instead of leaving a
+ dangling reference to the source object's #Tag field */
+ ExportedSong(ExportedSong &&src) noexcept
+ :LightSong(src, tag_buffer),
+ tag_buffer(std::move(src.tag_buffer)) {}
+
+ ExportedSong &operator=(ExportedSong &&) = delete;
};
#endif
diff --git a/src/song/LightSong.hxx b/src/song/LightSong.hxx
index ee372576a..06f0a41f9 100644
--- a/src/song/LightSong.hxx
+++ b/src/song/LightSong.hxx
@@ -88,6 +88,19 @@ struct LightSong {
LightSong(const char *_uri, const Tag &_tag) noexcept
:uri(_uri), tag(_tag) {}
+ /**
+ * A copy constructor which copies all fields, but only sets
+ * the tag to a caller-provided reference. This is used by
+ * the #ExportedSong move constructor.
+ */
+ LightSong(const LightSong &src, const Tag &_tag) noexcept
+ :directory(src.directory), uri(src.uri),
+ real_uri(src.real_uri),
+ tag(_tag),
+ mtime(src.mtime),
+ start_time(src.start_time), end_time(src.end_time),
+ audio_format(src.audio_format) {}
+
gcc_pure
std::string GetURI() const noexcept {
if (directory == nullptr)