db/simple: use class boost::intrusive::list

Makefile.am

@@ -370,8 +370,6 @@ libutil_a_SOURCES = \
 	src/util/PeakBuffer.cxx src/util/PeakBuffer.hxx \
 	src/util/OptionParser.cxx src/util/OptionParser.hxx \
 	src/util/OptionDef.hxx \
-	src/util/list.h \
-	src/util/list_sort.c src/util/list_sort.h \
 	src/util/ByteReverse.cxx src/util/ByteReverse.hxx \
 	src/util/bit_reverse.c src/util/bit_reverse.h
 

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 <assert.h>
 #include <string.h>
 #include <stdlib.h>
@@ -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;
 	}
 

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 <boost/intrusive/list.hpp>
 
 #include <string>
 
@@ -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<link_mode> LinkMode;
+	typedef boost::intrusive::list_member_hook<LinkMode> 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<Directory, Hook,
+					      &Directory::siblings> SiblingsHook;
+	typedef boost::intrusive::list<Directory, SiblingsHook,
+				       boost::intrusive::constant_time_size<false>> 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<typename T>
+	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<typename T>
+	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.
 	 *

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);
 

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 <boost/intrusive/list.hpp>
+
 #include <string>
 
 #include <assert.h>
@@ -39,6 +40,16 @@ class Storage;
  * #SimpleDatabase class.
  */
 struct Song {
+	static constexpr auto link_mode = boost::intrusive::normal_link;
+	typedef boost::intrusive::link_mode<link_mode> LinkMode;
+	typedef boost::intrusive::list_member_hook<LinkMode> 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<Song,
+			       boost::intrusive::member_hook<Song, Song::Hook,
+							     &Song::siblings>,
+			       boost::intrusive::constant_time_size<false>> SongList;
+
 #endif

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 <stdlib.h>
 
 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);
 }

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

src/db/update/Editor.cxx

@@ -64,15 +64,14 @@ DatabaseEditor::LockDeleteSong(Directory &parent, Song *song)
 inline void
 DatabaseEditor::ClearDirectory(Directory &directory)
 {
-	Directory *child, *n;
-	directory_for_each_child_safe(child, n, directory)
-		DeleteDirectory(child);
-
-	Song *song, *ns;
-	directory_for_each_song_safe(song, ns, directory) {
-		assert(song->parent == &directory);
-		DeleteSong(directory, song);
-	}
+	directory.ForEachChildSafe([this](Directory &child){
+			DeleteDirectory(&child);
+		});
+
+	directory.ForEachSongSafe([this, &directory](Song &song){
+			assert(song.parent == &directory);
+			DeleteSong(directory, &song);
+		});
 }
 
 void

src/db/update/Walk.cxx

@@ -80,26 +80,25 @@ UpdateWalk::RemoveExcludedFromDirectory(Directory &directory,
 {
 	db_lock();
 
-	Directory *child, *n;
-	directory_for_each_child_safe(child, n, directory) {
-		const auto name_fs = AllocatedPath::FromUTF8(child->GetName());
+	directory.ForEachChildSafe([&](Directory &child){
+			const auto name_fs =
+				AllocatedPath::FromUTF8(child.GetName());
 
-		if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
-			editor.DeleteDirectory(child);
-			modified = true;
-		}
-	}
+			if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
+				editor.DeleteDirectory(&child);
+				modified = true;
+			}
+		});
 
-	Song *song, *ns;
-	directory_for_each_song_safe(song, ns, directory) {
-		assert(song->parent == &directory);
+	directory.ForEachSongSafe([&](Song &song){
+			assert(song.parent == &directory);
 
-		const auto name_fs = AllocatedPath::FromUTF8(song->uri);
-		if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
-			editor.DeleteSong(directory, song);
-			modified = true;
-		}
-	}
+			const auto name_fs = AllocatedPath::FromUTF8(song.uri);
+			if (name_fs.IsNull() || exclude_list.Check(name_fs)) {
+				editor.DeleteSong(directory, &song);
+				modified = true;
+			}
+		});
 
 	db_unlock();
 }
@@ -107,25 +106,23 @@ UpdateWalk::RemoveExcludedFromDirectory(Directory &directory,
 inline void
 UpdateWalk::PurgeDeletedFromDirectory(Directory &directory)
 {
-	Directory *child, *n;
-	directory_for_each_child_safe(child, n, directory) {
-		if (DirectoryExists(storage, *child))
-			continue;
+	directory.ForEachChildSafe([&](Directory &child){
+			if (DirectoryExists(storage, child))
+				return;
 
-		editor.LockDeleteDirectory(child);
+			editor.LockDeleteDirectory(&child);
 
-		modified = true;
-	}
+			modified = true;
+		});
 
-	Song *song, *ns;
-	directory_for_each_song_safe(song, ns, directory) {
-		if (!directory_child_is_regular(storage, directory,
-						song->uri)) {
-			editor.LockDeleteSong(directory, song);
+	directory.ForEachSongSafe([&](Song &song){
+			if (!directory_child_is_regular(storage, directory,
+							song.uri)) {
+				editor.LockDeleteSong(directory, &song);
 
-			modified = true;
-		}
-	}
+				modified = true;
+			}
+		});
 
 	for (auto i = directory.playlists.begin(),
 		     end = directory.playlists.end();

src/util/list_sort.c

-/*
- * Copyright (C) 2003-2014 The Music Player Daemon Project
- * http://www.musicpd.org
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-/*
- * This code was imported from the Linux kernel.
- *
- */
-
-#include "list_sort.h"
-#include "list.h"
-#include "Macros.hxx"
-#include "Compiler.h"
-
-#include <string.h>
-
-#define unlikely gcc_unlikely
-
-#define MAX_LIST_LENGTH_BITS 20
-
-/*
- * Returns a list organized in an intermediate format suited
- * to chaining of merge() calls: null-terminated, no reserved or
- * sentinel head node, "prev" links not maintained.
- */
-static struct list_head *merge(void *priv,
-				int (*cmp)(void *priv, struct list_head *a,
-					struct list_head *b),
-				struct list_head *a, struct list_head *b)
-{
-	struct list_head head, *tail = &head;
-
-	while (a && b) {
-		/* if equal, take 'a' -- important for sort stability */
-		if ((*cmp)(priv, a, b) <= 0) {
-			tail->next = a;
-			a = a->next;
-		} else {
-			tail->next = b;
-			b = b->next;
-		}
-		tail = tail->next;
-	}
-	tail->next = a?a:b;
-	return head.next;
-}
-
-/*
- * Combine final list merge with restoration of standard doubly-linked
- * list structure.  This approach duplicates code from merge(), but
- * runs faster than the tidier alternatives of either a separate final
- * prev-link restoration pass, or maintaining the prev links
- * throughout.
- */
-static void merge_and_restore_back_links(void *priv,
-				int (*cmp)(void *priv, struct list_head *a,
-					struct list_head *b),
-				struct list_head *head,
-				struct list_head *a, struct list_head *b)
-{
-	struct list_head *tail = head;
-
-	while (a && b) {
-		/* if equal, take 'a' -- important for sort stability */
-		if ((*cmp)(priv, a, b) <= 0) {
-			tail->next = a;
-			a->prev = tail;
-			a = a->next;
-		} else {
-			tail->next = b;
-			b->prev = tail;
-			b = b->next;
-		}
-		tail = tail->next;
-	}
-	tail->next = a ? a : b;
-
-	do {
-		/*
-		 * In worst cases this loop may run many iterations.
-		 * Continue callbacks to the client even though no
-		 * element comparison is needed, so the client's cmp()
-		 * routine can invoke cond_resched() periodically.
-		 */
-		(*cmp)(priv, tail->next, tail->next);
-
-		tail->next->prev = tail;
-		tail = tail->next;
-	} while (tail->next);
-
-	tail->next = head;
-	head->prev = tail;
-}
-
-/**
- * list_sort - sort a list
- * @priv: private data, opaque to list_sort(), passed to @cmp
- * @head: the list to sort
- * @cmp: the elements comparison function
- *
- * This function implements "merge sort", which has O(nlog(n))
- * complexity.
- *
- * The comparison function @cmp must return a negative value if @a
- * should sort before @b, and a positive value if @a should sort after
- * @b. If @a and @b are equivalent, and their original relative
- * ordering is to be preserved, @cmp must return 0.
- */
-void list_sort(void *priv, struct list_head *head,
-		int (*cmp)(void *priv, struct list_head *a,
-			struct list_head *b))
-{
-	struct list_head *part[MAX_LIST_LENGTH_BITS+1]; /* sorted partial lists
-						-- last slot is a sentinel */
-	int lev;  /* index into part[] */
-	int max_lev = 0;
-	struct list_head *list;
-
-	if (list_empty(head))
-		return;
-
-	memset(part, 0, sizeof(part));
-
-	head->prev->next = NULL;
-	list = head->next;
-
-	while (list) {
-		struct list_head *cur = list;
-		list = list->next;
-		cur->next = NULL;
-
-		for (lev = 0; part[lev]; lev++) {
-			cur = merge(priv, cmp, part[lev], cur);
-			part[lev] = NULL;
-		}
-		if (lev > max_lev) {
-			max_lev = lev;
-		}
-		part[lev] = cur;
-	}
-
-	for (lev = 0; lev < max_lev; lev++)
-		if (part[lev])
-			list = merge(priv, cmp, part[lev], list);
-
-	merge_and_restore_back_links(priv, cmp, head, part[max_lev], list);
-}

src/util/list_sort.h

-/*
- * Copyright (C) 2003-2014 The Music Player Daemon Project
- * http://www.musicpd.org
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-/*
- * This code was imported from the Linux kernel.
- *
- */
-
-#ifndef _LINUX_LIST_SORT_H
-#define _LINUX_LIST_SORT_H
-
-struct list_head;
-
-void list_sort(void *priv, struct list_head *head,
-	       int (*cmp)(void *priv, struct list_head *a,
-			  struct list_head *b));
-#endif