From 3ca0a39a357d9be9c85de4892ac02716f8af2ae8 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 10 Jun 2014 21:15:40 +0200 Subject: db/simple: use class boost::intrusive::list Remove the C list_head library and use type-safe C++ instead. --- src/db/plugins/simple/Directory.cxx | 90 +++++++++++++-------------------- src/db/plugins/simple/Directory.hxx | 60 +++++++++++++++------- src/db/plugins/simple/DirectorySave.cxx | 14 +++-- src/db/plugins/simple/Song.hxx | 20 +++++++- src/db/plugins/simple/SongSort.cxx | 29 +++++------ src/db/plugins/simple/SongSort.hxx | 4 +- 6 files changed, 114 insertions(+), 103 deletions(-) (limited to 'src/db/plugins') diff --git a/src/db/plugins/simple/Directory.cxx b/src/db/plugins/simple/Directory.cxx index 3ac2f96a2..6259df49f 100644 --- a/src/db/plugins/simple/Directory.cxx +++ b/src/db/plugins/simple/Directory.cxx @@ -33,10 +33,6 @@ #include "util/Alloc.hxx" #include "util/Error.hxx" -extern "C" { -#include "util/list_sort.h" -} - #include #include #include @@ -47,21 +43,14 @@ Directory::Directory(std::string &&_path_utf8, Directory *_parent) path(std::move(_path_utf8)), mounted_database(nullptr) { - INIT_LIST_HEAD(&children); - INIT_LIST_HEAD(&songs); } Directory::~Directory() { delete mounted_database; - Song *song, *ns; - directory_for_each_song_safe(song, ns, *this) - song->Free(); - - Directory *child, *n; - directory_for_each_child_safe(child, n, *this) - delete child; + songs.clear_and_dispose(Song::Disposer()); + children.clear_and_dispose(Disposer()); } void @@ -70,8 +59,8 @@ Directory::Delete() assert(holding_db_lock()); assert(parent != nullptr); - list_del(&siblings); - delete this; + parent->children.erase_and_dispose(parent->children.iterator_to(*this), + Disposer()); } const char * @@ -94,7 +83,7 @@ Directory::CreateChild(const char *name_utf8) : PathTraitsUTF8::Build(GetPath(), name_utf8); Directory *child = new Directory(std::move(path_utf8), this); - list_add_tail(&child->siblings, &children); + children.push_back(*child); return child; } @@ -103,10 +92,9 @@ Directory::FindChild(const char *name) const { assert(holding_db_lock()); - const Directory *child; - directory_for_each_child(child, *this) - if (strcmp(child->GetName(), name) == 0) - return child; + for (const auto &child : children) + if (strcmp(child.GetName(), name) == 0) + return &child; return nullptr; } @@ -116,17 +104,14 @@ Directory::PruneEmpty() { assert(holding_db_lock()); - Directory *child, *n; - directory_for_each_child_safe(child, n, *this) { - if (child->IsMount()) - /* never prune mount points; they're always - empty by definition, but that's ok */ - continue; - + for (auto child = children.begin(), end = children.end(); + child != end;) { child->PruneEmpty(); if (child->IsEmpty()) - child->Delete(); + child = children.erase_and_dispose(child, Disposer()); + else + ++child; } } @@ -182,7 +167,7 @@ Directory::AddSong(Song *song) assert(song != nullptr); assert(song->parent == this); - list_add_tail(&song->siblings, &songs); + songs.push_back(*song); } void @@ -192,7 +177,7 @@ Directory::RemoveSong(Song *song) assert(song != nullptr); assert(song->parent == this); - list_del(&song->siblings); + songs.erase(songs.iterator_to(*song)); } const Song * @@ -201,25 +186,21 @@ Directory::FindSong(const char *name_utf8) const assert(holding_db_lock()); assert(name_utf8 != nullptr); - Song *song; - directory_for_each_song(song, *this) { - assert(song->parent == this); + for (auto &song : songs) { + assert(song.parent == this); - if (strcmp(song->uri, name_utf8) == 0) - return song; + if (strcmp(song.uri, name_utf8) == 0) + return &song; } return nullptr; } -static int -directory_cmp(gcc_unused void *priv, - struct list_head *_a, struct list_head *_b) +gcc_pure +static bool +directory_cmp(const Directory &a, const Directory &b) { - const Directory *a = (const Directory *)_a; - const Directory *b = (const Directory *)_b; - - return IcuCollate(a->path.c_str(), b->path.c_str()); + return IcuCollate(a.path.c_str(), b.path.c_str()) < 0; } void @@ -227,12 +208,11 @@ Directory::Sort() { assert(holding_db_lock()); - list_sort(nullptr, &children, directory_cmp); - song_list_sort(&songs); + children.sort(directory_cmp); + song_list_sort(songs); - Directory *child; - directory_for_each_child(child, *this) - child->Sort(); + for (auto &child : children) + child.Sort(); } bool @@ -260,9 +240,8 @@ Directory::Walk(bool recursive, const SongFilter *filter, } if (visit_song) { - Song *song; - directory_for_each_song(song, *this) { - const LightSong song2 = song->Export(); + for (auto &song : songs){ + const LightSong song2 = song.Export(); if ((filter == nullptr || filter->Match(song2)) && !visit_song(song2, error)) return false; @@ -275,16 +254,15 @@ Directory::Walk(bool recursive, const SongFilter *filter, return false; } - Directory *child; - directory_for_each_child(child, *this) { + for (auto &child : children) { if (visit_directory && - !visit_directory(child->Export(), error)) + !visit_directory(child.Export(), error)) return false; if (recursive && - !child->Walk(recursive, filter, - visit_directory, visit_song, visit_playlist, - error)) + !child.Walk(recursive, filter, + visit_directory, visit_song, visit_playlist, + error)) return false; } diff --git a/src/db/plugins/simple/Directory.hxx b/src/db/plugins/simple/Directory.hxx index 029815d95..80675bd21 100644 --- a/src/db/plugins/simple/Directory.hxx +++ b/src/db/plugins/simple/Directory.hxx @@ -21,10 +21,12 @@ #define MPD_DIRECTORY_HXX #include "check.h" -#include "util/list.h" #include "Compiler.h" #include "db/Visitor.hxx" #include "db/PlaylistVector.hxx" +#include "Song.hxx" + +#include #include @@ -41,25 +43,22 @@ static constexpr unsigned DEVICE_INARCHIVE = -1; */ static constexpr unsigned DEVICE_CONTAINER = -2; -#define directory_for_each_child(pos, directory) \ - list_for_each_entry(pos, &(directory).children, siblings) - -#define directory_for_each_child_safe(pos, n, directory) \ - list_for_each_entry_safe(pos, n, &(directory).children, siblings) - -#define directory_for_each_song(pos, directory) \ - list_for_each_entry(pos, &(directory).songs, siblings) - -#define directory_for_each_song_safe(pos, n, directory) \ - list_for_each_entry_safe(pos, n, &(directory).songs, siblings) - -struct Song; struct db_visitor; class SongFilter; class Error; class Database; struct Directory { + static constexpr auto link_mode = boost::intrusive::normal_link; + typedef boost::intrusive::link_mode LinkMode; + typedef boost::intrusive::list_member_hook Hook; + + struct Disposer { + void operator()(Directory *directory) const { + delete directory; + } + }; + /** * Pointers to the siblings of this directory within the * parent directory. It is unused (undefined) in the root @@ -68,7 +67,12 @@ struct Directory { * This attribute is protected with the global #db_mutex. * Read access in the update thread does not need protection. */ - struct list_head siblings; + Hook siblings; + + typedef boost::intrusive::member_hook SiblingsHook; + typedef boost::intrusive::list> List; /** * A doubly linked list of child directories. @@ -76,7 +80,7 @@ struct Directory { * This attribute is protected with the global #db_mutex. * Read access in the update thread does not need protection. */ - struct list_head children; + List children; /** * A doubly linked list of songs within this directory. @@ -84,7 +88,7 @@ struct Directory { * This attribute is protected with the global #db_mutex. * Read access in the update thread does not need protection. */ - struct list_head songs; + SongList songs; PlaylistVector playlists; @@ -186,8 +190,8 @@ public: gcc_pure bool IsEmpty() const { - return list_empty(&children) && - list_empty(&songs) && + return children.empty() && + songs.empty() && playlists.empty(); } @@ -210,6 +214,24 @@ public: return parent == nullptr; } + template + void ForEachChildSafe(T &&t) { + const auto end = children.end(); + for (auto i = children.begin(), next = i; i != end; i = next) { + next = std::next(i); + t(*i); + } + } + + template + void ForEachSongSafe(T &&t) { + const auto end = songs.end(); + for (auto i = songs.begin(), next = i; i != end; i = next) { + next = std::next(i); + t(*i); + } + } + /** * Look up a song in this directory by its name. * diff --git a/src/db/plugins/simple/DirectorySave.cxx b/src/db/plugins/simple/DirectorySave.cxx index b5989dd06..9d3ebbac2 100644 --- a/src/db/plugins/simple/DirectorySave.cxx +++ b/src/db/plugins/simple/DirectorySave.cxx @@ -84,20 +84,18 @@ directory_save(FILE *fp, const Directory &directory) fprintf(fp, "%s%s\n", DIRECTORY_BEGIN, directory.GetPath()); } - Directory *cur; - directory_for_each_child(cur, directory) { - fprintf(fp, DIRECTORY_DIR "%s\n", cur->GetName()); + for (const auto &child : directory.children) { + fprintf(fp, DIRECTORY_DIR "%s\n", child.GetName()); - if (!cur->IsMount()) - directory_save(fp, *cur); + if (!child.IsMount()) + directory_save(fp, child); if (ferror(fp)) return; } - Song *song; - directory_for_each_song(song, directory) - song_save(fp, *song); + for (const auto &song : directory.songs) + song_save(fp, song); playlist_vector_save(fp, directory.playlists); diff --git a/src/db/plugins/simple/Song.hxx b/src/db/plugins/simple/Song.hxx index 75fce20e9..b2e85aa6b 100644 --- a/src/db/plugins/simple/Song.hxx +++ b/src/db/plugins/simple/Song.hxx @@ -20,10 +20,11 @@ #ifndef MPD_SONG_HXX #define MPD_SONG_HXX -#include "util/list.h" #include "tag/Tag.hxx" #include "Compiler.h" +#include + #include #include @@ -39,6 +40,16 @@ class Storage; * #SimpleDatabase class. */ struct Song { + static constexpr auto link_mode = boost::intrusive::normal_link; + typedef boost::intrusive::link_mode LinkMode; + typedef boost::intrusive::list_member_hook Hook; + + struct Disposer { + void operator()(Song *song) const { + song->Free(); + } + }; + /** * Pointers to the siblings of this directory within the * parent directory. It is unused (undefined) if this song is @@ -47,7 +58,7 @@ struct Song { * This attribute is protected with the global #db_mutex. * Read access in the update thread does not need protection. */ - struct list_head siblings; + Hook siblings; Tag tag; @@ -110,4 +121,9 @@ struct Song { LightSong Export() const; }; +typedef boost::intrusive::list, + boost::intrusive::constant_time_size> SongList; + #endif diff --git a/src/db/plugins/simple/SongSort.cxx b/src/db/plugins/simple/SongSort.cxx index c5752f568..4b7144937 100644 --- a/src/db/plugins/simple/SongSort.cxx +++ b/src/db/plugins/simple/SongSort.cxx @@ -23,10 +23,6 @@ #include "tag/Tag.hxx" #include "lib/icu/Collate.hxx" -extern "C" { -#include "util/list_sort.h" -} - #include static int @@ -80,34 +76,33 @@ compare_tag_item(const Tag &a, const Tag &b, TagType type) } /* Only used for sorting/searchin a songvec, not general purpose compares */ -static int -song_cmp(gcc_unused void *priv, struct list_head *_a, struct list_head *_b) +gcc_pure +static bool +song_cmp(const Song &a, const Song &b) { - const Song *a = (const Song *)_a; - const Song *b = (const Song *)_b; int ret; /* first sort by album */ - ret = compare_string_tag_item(a->tag, b->tag, TAG_ALBUM); + ret = compare_string_tag_item(a.tag, b.tag, TAG_ALBUM); if (ret != 0) - return ret; + return ret < 0; /* then sort by disc */ - ret = compare_tag_item(a->tag, b->tag, TAG_DISC); + ret = compare_tag_item(a.tag, b.tag, TAG_DISC); if (ret != 0) - return ret; + return ret < 0; /* then by track number */ - ret = compare_tag_item(a->tag, b->tag, TAG_TRACK); + ret = compare_tag_item(a.tag, b.tag, TAG_TRACK); if (ret != 0) - return ret; + return ret < 0; /* still no difference? compare file name */ - return IcuCollate(a->uri, b->uri); + return IcuCollate(a.uri, b.uri) < 0; } void -song_list_sort(struct list_head *songs) +song_list_sort(SongList &songs) { - list_sort(nullptr, songs, song_cmp); + songs.sort(song_cmp); } diff --git a/src/db/plugins/simple/SongSort.hxx b/src/db/plugins/simple/SongSort.hxx index 28b903532..2a0c4383b 100644 --- a/src/db/plugins/simple/SongSort.hxx +++ b/src/db/plugins/simple/SongSort.hxx @@ -20,9 +20,11 @@ #ifndef MPD_SONG_SORT_HXX #define MPD_SONG_SORT_HXX +#include "Song.hxx" + struct list_head; void -song_list_sort(list_head *songs); +song_list_sort(SongList &songs); #endif -- cgit v1.2.3