From dcaa1cafa42d14595068f9434483d151bf6b7cb7 Mon Sep 17 00:00:00 2001 From: Andrzej Rybczak Date: Sat, 30 Aug 2014 02:30:54 +0200 Subject: [PATCH] playlist: make kept song count collision resistant --- src/browser.cpp | 13 ++----------- src/media_library.cpp | 4 ++-- src/playlist.cpp | 14 +++++++------- src/playlist.h | 6 +++--- src/song.cpp | 10 ++-------- src/song.h | 20 +++++++++++++++++--- src/status.cpp | 6 +++--- src/tag_editor.cpp | 2 +- 8 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/browser.cpp b/src/browser.cpp index 5844eea8..ab476267 100644 --- a/src/browser.cpp +++ b/src/browser.cpp @@ -398,7 +398,7 @@ void Browser::LocateSong(const MPD::Song &s) GetDirectory(s.getDirectory()); for (size_t i = 0; i < w.size(); ++i) { - if (w[i].value().type == itSong && s.getHash() == w[i].value().song->getHash()) + if (w[i].value().type == itSong && s == *w[i].value().song) { w.highlight(i); break; @@ -460,16 +460,7 @@ void Browser::GetDirectory(std::string dir, std::string subdir) } case itSong: { - bool bold = 0; - for (size_t i = 0; i < myPlaylist->main().size(); ++i) - { - if (myPlaylist->main().at(i).value().getHash() == it->song->getHash()) - { - bold = 1; - break; - } - } - w.addItem(*it, bold); + w.addItem(*it, myPlaylist->checkForSong(*it->song)); break; } } diff --git a/src/media_library.cpp b/src/media_library.cpp index 15776539..76e89fa3 100644 --- a/src/media_library.cpp +++ b/src/media_library.cpp @@ -980,11 +980,11 @@ void MediaLibrary::LocateSong(const MPD::Song &s) if (Songs.empty()) update(); - if (s.getHash() != Songs.current().value().getHash()) + if (s != Songs.current().value()) { for (size_t i = 0; i < Songs.size(); ++i) { - if (s.getHash() == Songs[i].value().getHash()) + if (s == Songs[i].value()) { Songs.highlight(i); break; diff --git a/src/playlist.cpp b/src/playlist.cpp index de047d00..1a603729 100644 --- a/src/playlist.cpp +++ b/src/playlist.cpp @@ -360,20 +360,20 @@ unsigned Playlist::currentSongLength() const bool Playlist::checkForSong(const MPD::Song &s) { - return m_song_hashes.find(s.getHash()) != m_song_hashes.end(); + return m_song_refs.find(s) != m_song_refs.end(); } -void Playlist::registerHash(size_t hash) +void Playlist::registerSong(const MPD::Song &s) { - ++m_song_hashes[hash]; + ++m_song_refs[s]; } -void Playlist::unregisterHash(size_t hash) +void Playlist::unregisterSong(const MPD::Song &s) { - auto it = m_song_hashes.find(hash); - assert(it != m_song_hashes.end()); + auto it = m_song_refs.find(s); + assert(it != m_song_refs.end()); if (it->second == 1) - m_song_hashes.erase(it); + m_song_refs.erase(it); else --it->second; } diff --git a/src/playlist.h b/src/playlist.h index 69f875e1..33f3733a 100644 --- a/src/playlist.h +++ b/src/playlist.h @@ -81,8 +81,8 @@ struct Playlist: Screen>, Filterable, HasSongs, Searchable, unsigned currentSongLength() const; bool checkForSong(const MPD::Song &s); - void registerHash(size_t hash); - void unregisterHash(size_t hash); + void registerSong(const MPD::Song &s); + void unregisterSong(const MPD::Song &s); void reloadTotalLength() { m_reload_total_length = true; } void reloadRemaining() { m_reload_remaining = true; } @@ -95,7 +95,7 @@ private: std::string m_stats; - std::unordered_map m_song_hashes; + std::unordered_map m_song_refs; size_t m_total_length;; size_t m_remaining_time; diff --git a/src/song.cpp b/src/song.cpp index 78874130..c7d07ba7 100644 --- a/src/song.cpp +++ b/src/song.cpp @@ -30,13 +30,13 @@ #include "utility/wide_string.h" #include "window.h" -namespace {// +namespace { size_t calc_hash(const char* s, unsigned seed = 0) { size_t hash = seed; while (*s) - hash = hash * 101 + *s++; + hash = hash * 101 + *s++; return hash; } @@ -211,12 +211,6 @@ std::string MPD::Song::getTags(GetFunction f, const std::string &tags_separator) return result; } -unsigned Song::getHash() const -{ - assert(m_song); - return m_hash; -} - unsigned Song::getDuration() const { assert(m_song); diff --git a/src/song.h b/src/song.h index f97599fe..c8e57d46 100644 --- a/src/song.h +++ b/src/song.h @@ -21,6 +21,7 @@ #ifndef NCMPCPP_SONG_H #define NCMPCPP_SONG_H +#include #include #include #include @@ -32,6 +33,10 @@ namespace MPD {// struct Song { + struct Hash { + size_t operator()(const Song &s) const { return s.m_hash; } + }; + typedef std::string (Song::*GetFunction)(unsigned) const; Song() { } @@ -61,7 +66,6 @@ struct Song virtual std::string getTags(GetFunction f, const std::string &tags_separator) const; - virtual unsigned getHash() const; virtual unsigned getDuration() const; virtual unsigned getPosition() const; virtual unsigned getID() const; @@ -76,9 +80,19 @@ struct Song virtual std::string toString(const std::string &fmt, const std::string &tags_separator, const std::string &escape_chars = "") const; - bool operator==(const Song &rhs) const { return m_hash == rhs.m_hash; } - bool operator!=(const Song &rhs) const { return m_hash != rhs.m_hash; } + bool operator==(const Song &rhs) const { + if (m_hash != rhs.m_hash) + return false; + return strcmp(c_uri(), rhs.c_uri()) == 0; + } + bool operator!=(const Song &rhs) const { + if (m_hash != rhs.m_hash) + return true; + return strcmp(c_uri(), rhs.c_uri()) != 0; + } + const char *c_uri() const { return m_song ? mpd_song_get_uri(m_song.get()) : ""; } + static std::string ShowTime(unsigned length); static void validateFormat(const std::string &fmt); diff --git a/src/status.cpp b/src/status.cpp index 3b203f17..1e6a25d4 100644 --- a/src/status.cpp +++ b/src/status.cpp @@ -257,7 +257,7 @@ void Status::Changes::playlist() auto it = myPlaylist->main().begin()+playlist_length; auto end = myPlaylist->main().end(); for (; it != end; ++it) - myPlaylist->unregisterHash(it->value().getHash()); + myPlaylist->unregisterSong(it->value()); myPlaylist->main().resizeList(playlist_length); } @@ -267,12 +267,12 @@ void Status::Changes::playlist() { // if song's already in playlist, replace it with a new one MPD::Song &old_s = myPlaylist->main()[pos].value(); - myPlaylist->unregisterHash(old_s.getHash()); + myPlaylist->unregisterSong(old_s); old_s = s; } else // otherwise just add it to playlist myPlaylist->main().addItem(s); - myPlaylist->registerHash(s.getHash()); + myPlaylist->registerSong(s); }); }); diff --git a/src/tag_editor.cpp b/src/tag_editor.cpp index 9d9476b4..2655eb66 100644 --- a/src/tag_editor.cpp +++ b/src/tag_editor.cpp @@ -1025,7 +1025,7 @@ void TagEditor::LocateSong(const MPD::Song &s) // highlight our file for (size_t i = 0; i < Tags->size(); ++i) { - if ((*Tags)[i].value().getHash() == s.getHash()) + if ((*Tags)[i].value() == s) { Tags->highlight(i); break;