From 3402a7c25566176da698cd90ad166e93a2560e3d Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 20:05:31 -0700
Subject: gcc: define mpd_sizeof_str_flex_array

This can save a few bytes here on newer gcc and there and will
hopefully make it more obvious what we're doing with
that last struct element
---
 src/gcc.h | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'src')

diff --git a/src/gcc.h b/src/gcc.h
index 7492e6cad..6bd85663d 100644
--- a/src/gcc.h
+++ b/src/gcc.h
@@ -32,6 +32,10 @@
 #  define mpd_must_check	__attribute__ ((warn_unused_result))
 #  define mpd_noreturn		__attribute__ ((noreturn))
 #  define mpd_packed		__attribute__ ((packed))
+
+/* str_flex_array is always >= 1 because we always 0 terminate strings */
+#  define mpd_sizeof_str_flex_array	1
+
 /* these are very useful for type checking */
 #  define mpd_printf		__attribute__ ((format(printf,1,2)))
 #  define mpd_fprintf		__attribute__ ((format(printf,2,3)))
@@ -52,6 +56,7 @@
 #  define mpd_must_check
 #  define mpd_noreturn
 #  define mpd_packed
+#  define mpd_sizeof_str_flex_array	(sizeof(size_t))
 #  define mpd_printf
 #  define mpd_fprintf
 #  define mpd_fprintf_
-- 
cgit v1.2.3


From 442b041d3f00e2edec209dfafecb50f49f03402c Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 20:07:24 -0700
Subject: song: use mpd_sizeof_str_flex_array for song.url

This can potentially save a few bytes on 64-bit
architectures and makes it more obvious what
we're doing.
---
 src/song.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'src')

diff --git a/src/song.h b/src/song.h
index 338bcf6c1..e29c034ef 100644
--- a/src/song.h
+++ b/src/song.h
@@ -35,8 +35,8 @@ struct mpd_song {
 	struct mpd_tag *tag;
 	struct directory *parent;
 	time_t mtime;
-	char url[sizeof(size_t)];
-};
+	char url[mpd_sizeof_str_flex_array];
+} mpd_packed;
 
 void song_free(struct mpd_song *);
 
-- 
cgit v1.2.3


From aed02f0fedf56f6684cb98e33d0c616d4f80e81f Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 20:15:43 -0700
Subject: tag_item: avoid wasting space when struct is unpackable

Not all compilers support struct packing, and those that don't
shouldn't be punished for it.
---
 src/tag.h      | 2 +-
 src/tag_pool.c | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

(limited to 'src')

diff --git a/src/tag.h b/src/tag.h
index 744b82e83..58df5b3d1 100644
--- a/src/tag.h
+++ b/src/tag.h
@@ -42,7 +42,7 @@ extern const char *mpdTagItemKeys[];
 
 struct tag_item {
 	enum tag_type type;
-	char value[1];
+	char value[mpd_sizeof_str_flex_array];
 } mpd_packed;
 
 struct mpd_tag {
diff --git a/src/tag_pool.c b/src/tag_pool.c
index d227a2988..1d92502cc 100644
--- a/src/tag_pool.c
+++ b/src/tag_pool.c
@@ -67,7 +67,9 @@ static struct slot *slot_alloc(struct slot *next,
 			       enum tag_type type,
 			       const char *value, int length)
 {
-	struct slot *slot = xmalloc(sizeof(*slot) + length);
+	struct slot *slot;
+
+	slot = xmalloc(sizeof(*slot) - sizeof(slot->item.value) + length + 1);
 	slot->next = next;
 	slot->ref = 1;
 	slot->item.type = type;
-- 
cgit v1.2.3


From c6ee14dca5d7336dad20a6057aa2a8006d4447b8 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 20:24:26 -0700
Subject: directory: use mpd_sizeof_str_flex_array for path, too

This way we avoid unnecessary heap allocations.
---
 src/directory.c | 12 ++++++------
 src/directory.h |  5 +++--
 2 files changed, 9 insertions(+), 8 deletions(-)

(limited to 'src')

diff --git a/src/directory.c b/src/directory.c
index b90b477fd..699b6fbc1 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -23,15 +23,16 @@
 #include "myfprintf.h"
 #include "dirvec.h"
 
-struct directory * directory_new(const char *dirname, struct directory * parent)
+struct directory * directory_new(const char *path, struct directory * parent)
 {
 	struct directory *dir;
+	size_t pathlen = strlen(path);
 
-	assert(dirname != NULL);
-	assert((*dirname == 0) == (parent == NULL));
+	assert(path != NULL);
+	assert((*path == 0) == (parent == NULL));
 
-	dir = xcalloc(1, sizeof(struct directory));
-	dir->path = xstrdup(dirname);
+	dir = xcalloc(1, sizeof(*dir) - sizeof(dir->path) + pathlen + 1);
+	memcpy(dir->path, path, pathlen + 1);
 	dir->parent = parent;
 
 	return dir;
@@ -41,7 +42,6 @@ void directory_free(struct directory *dir)
 {
 	dirvec_destroy(&dir->children);
 	songvec_destroy(&dir->songs);
-	free(dir->path);
 	free(dir);
 	/* this resets last dir returned */
 	/*directory_get_path(NULL); */
diff --git a/src/directory.h b/src/directory.h
index 949df0c0e..546e55183 100644
--- a/src/directory.h
+++ b/src/directory.h
@@ -22,6 +22,7 @@
 #include "song.h"
 #include "dirvec.h"
 #include "songvec.h"
+#include "gcc.h"
 
 #define DIRECTORY_DIR		"directory: "
 #define DIRECTORY_MTIME		"mtime: " /* DEPRECATED, noop-read-only */
@@ -33,14 +34,14 @@
 #define DIRECTORY_FS_CHARSET	"fs_charset: "
 
 struct directory {
-	char *path;
 	struct dirvec children;
 	struct songvec songs;
 	struct directory *parent;
 	ino_t inode;
 	dev_t device;
 	unsigned stat; /* not needed if ino_t == dev_t == 0 is impossible */
-};
+	char path[mpd_sizeof_str_flex_array];
+} mpd_packed;
 
 static inline int isRootDirectory(const char *name)
 {
-- 
cgit v1.2.3


From e28fb29349a811f0f306e9ac4bedf6013417d043 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 20:41:55 -0700
Subject: directory: make music_root global and avoid runtime initialization

mpd can't function without music_root; so don't bother
allocating it on the heap nor checking to see if it's
initialized.

Don't allow directory_new() to create a directory w/o a parent
or with an empty path, either: root is root and there can
be only one</highlander>.
---
 src/database.c  | 28 ++++++----------------------
 src/directory.c | 15 +++++++++------
 src/directory.h |  2 ++
 src/update.c    | 10 ++++------
 4 files changed, 21 insertions(+), 34 deletions(-)

(limited to 'src')

diff --git a/src/database.c b/src/database.c
index dde57ce6a..a9e80f425 100644
--- a/src/database.c
+++ b/src/database.c
@@ -32,14 +32,10 @@
 #include "os_compat.h"
 #include "myfprintf.h"
 
-static struct directory *music_root;
-
 static time_t directory_dbModTime;
 
 void db_init(void)
 {
-	music_root = directory_new("", NULL);
-
 	if (!directory_update_init(NULL))
 		FATAL("directory update failed\n");
 
@@ -54,22 +50,12 @@ void db_init(void)
 
 void db_finish(void)
 {
-	directory_free(music_root);
-}
-
-struct directory * db_get_root(void)
-{
-	assert(music_root != NULL);
-
-	return music_root;
+	directory_free(&music_root);
 }
 
 struct directory * db_get_directory(const char *name)
 {
-	if (name == NULL)
-		return music_root;
-
-	return directory_get_subdir(music_root, name);
+	return name ? directory_get_subdir(&music_root, name) : &music_root;
 }
 
 struct mpd_song *db_get_song(const char *file)
@@ -195,11 +181,11 @@ int db_save(void)
 	struct stat st;
 
 	DEBUG("removing empty directories from DB\n");
-	directory_prune_empty(music_root);
+	directory_prune_empty(&music_root);
 
 	DEBUG("sorting DB\n");
 
-	directory_sort(music_root);
+	directory_sort(&music_root);
 
 	DEBUG("writing DB\n");
 
@@ -220,7 +206,7 @@ int db_save(void)
 	         DIRECTORY_FS_CHARSET "%s\n"
 	         DIRECTORY_INFO_END "\n", getFsCharset());
 
-	if (directory_save(fd, music_root) < 0) {
+	if (directory_save(fd, &music_root) < 0) {
 		ERROR("Failed to write to database file: %s\n",
 		      strerror(errno));
 		xclose(fd);
@@ -243,8 +229,6 @@ int db_load(void)
 	int foundFsCharset = 0;
 	int foundVersion = 0;
 
-	if (!music_root)
-		music_root = directory_new("", NULL);
 	while (!(fp = fopen(dbFile, "r")) && errno == EINTR) ;
 	if (fp == NULL) {
 		ERROR("unable to open db file \"%s\": %s\n",
@@ -296,7 +280,7 @@ int db_load(void)
 
 	DEBUG("reading DB\n");
 
-	directory_load(fp, music_root);
+	directory_load(fp, &music_root);
 	while (fclose(fp) && errno == EINTR) ;
 
 	stats.numberOfSongs = countSongsIn(NULL);
diff --git a/src/directory.c b/src/directory.c
index 699b6fbc1..846f2510a 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -23,14 +23,18 @@
 #include "myfprintf.h"
 #include "dirvec.h"
 
+struct directory music_root;
+
 struct directory * directory_new(const char *path, struct directory * parent)
 {
 	struct directory *dir;
-	size_t pathlen = strlen(path);
+	size_t pathlen;
 
-	assert(path != NULL);
-	assert((*path == 0) == (parent == NULL));
+	assert(path);
+	assert(*path);
+	assert(parent);
 
+	pathlen = strlen(path);
 	dir = xcalloc(1, sizeof(*dir) - sizeof(dir->path) + pathlen + 1);
 	memcpy(dir->path, path, pathlen + 1);
 	dir->parent = parent;
@@ -42,9 +46,8 @@ void directory_free(struct directory *dir)
 {
 	dirvec_destroy(&dir->children);
 	songvec_destroy(&dir->songs);
-	free(dir);
-	/* this resets last dir returned */
-	/*directory_get_path(NULL); */
+	if (dir != &music_root)
+		free(dir);
 }
 
 void directory_prune_empty(struct directory *dir)
diff --git a/src/directory.h b/src/directory.h
index 546e55183..9c3063d75 100644
--- a/src/directory.h
+++ b/src/directory.h
@@ -43,6 +43,8 @@ struct directory {
 	char path[mpd_sizeof_str_flex_array];
 } mpd_packed;
 
+extern struct directory music_root;
+
 static inline int isRootDirectory(const char *name)
 {
 	/* TODO: verify and remove !name check */
diff --git a/src/update.c b/src/update.c
index ca3640b73..3f6fa15ed 100644
--- a/src/update.c
+++ b/src/update.c
@@ -317,10 +317,9 @@ directory_make_child_checked(struct directory *parent, const char *path)
 	return dir;
 }
 
-static struct directory *
-addParentPathToDB(const char *utf8path)
+static struct directory * addParentPathToDB(const char *utf8path)
 {
-	struct directory *dir = db_get_root();
+	struct directory *dir = &music_root;
 	char *duplicated = xstrdup(utf8path);
 	char *slash = duplicated;
 
@@ -354,11 +353,10 @@ static void * update_task(void *_path)
 		updatePath((char *)_path);
 		free(_path);
 	} else {
-		struct directory *dir = db_get_root();
 		struct stat st;
 
-		if (myStat(directory_get_path(dir), &st) == 0)
-			updateDirectory(dir, &st);
+		if (myStat(directory_get_path(&music_root), &st) == 0)
+			updateDirectory(&music_root, &st);
 	}
 
 	if (modified)
-- 
cgit v1.2.3


From 8636b3185268e8de90c566d472c52b814111b8c3 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 21:09:08 -0700
Subject: Avoid calling isRootDirectory when we have a directory object

There is only one music_root and we can just compare
addresses.
---
 src/command.c        | 2 +-
 src/dbUtils.c        | 4 ++--
 src/directory_save.c | 4 ++--
 src/update.c         | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

(limited to 'src')

diff --git a/src/command.c b/src/command.c
index 518bf5f0d..f583f82a1 100644
--- a/src/command.c
+++ b/src/command.c
@@ -571,7 +571,7 @@ static int handleLsInfo(int fd, mpd_unused int *permission,
 
 	directory_print(fd, dir);
 
-	if (isRootDirectory(path))
+	if (dir == &music_root)
 		return lsPlaylists(fd, path);
 
 	return 0;
diff --git a/src/dbUtils.c b/src/dbUtils.c
index 1fadb232e..418020d9e 100644
--- a/src/dbUtils.c
+++ b/src/dbUtils.c
@@ -57,7 +57,7 @@ static int countSongsInDirectory(struct directory *dir, void *data)
 static int printDirectoryInDirectory(struct directory *dir, void *data)
 {
 	int fd = (int)(size_t)data;
-	if (!isRootDirectory(dir->path))
+	if (dir != &music_root)
 		fdprintf(fd, "directory: %s\n", directory_get_path(dir));
 	return 0;
 }
@@ -339,7 +339,7 @@ static int sumSavedFilenameMemoryInDirectory(struct directory *dir, void *data)
 {
 	int *sum = data;
 
-	if (!isRootDirectory(dir->path))
+	if (dir != &music_root)
 		return 0;
 
 	*sum += (strlen(directory_get_path(dir)) + 1
diff --git a/src/directory_save.c b/src/directory_save.c
index c39ece58a..e12de0019 100644
--- a/src/directory_save.c
+++ b/src/directory_save.c
@@ -45,7 +45,7 @@ int directory_save(int fd, struct directory *dir)
 	struct dirvec *children = &dir->children;
 	size_t i;
 
-	if (!isRootDirectory(dir->path) &&
+	if ((dir != &music_root) &&
 	    fdprintf(fd, DIRECTORY_BEGIN "%s\n",
 	             directory_get_path(dir)) < 0)
 		return -1;
@@ -70,7 +70,7 @@ int directory_save(int fd, struct directory *dir)
 	if (fdprintf(fd, SONG_END "\n") < 0)
 		return -1;
 
-	if (!isRootDirectory(dir->path) &&
+	if ((dir != &music_root) &&
 	    fdprintf(fd, DIRECTORY_END "%s\n",
 	             directory_get_path(dir)) < 0)
 		return -1;
diff --git a/src/update.c b/src/update.c
index 3f6fa15ed..7c2a7570a 100644
--- a/src/update.c
+++ b/src/update.c
@@ -278,7 +278,7 @@ static int updateDirectory(struct directory *dir, const struct stat *st)
 		if (!utf8)
 			continue;
 
-		if (!isRootDirectory(dir->path))
+		if (dir != &music_root)
 			utf8 = pfx_dir(path_max_tmp, utf8, strlen(utf8),
 			               dirname, strlen(dirname));
 
-- 
cgit v1.2.3


From 0fd45f50ec56bcfacae3f6f141ea804b9ac3493f Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 21:31:26 -0700
Subject: directory: rename isRootDirectory => path_is_music_root

down with camelCase!
---
 src/directory.c | 2 +-
 src/directory.h | 6 +++---
 src/update.c    | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

(limited to 'src')

diff --git a/src/directory.c b/src/directory.c
index 846f2510a..e66349e02 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -74,7 +74,7 @@ directory_get_subdir(struct directory *dir, const char *name)
 
 	assert(name != NULL);
 
-	if (isRootDirectory(name))
+	if (path_is_music_root(name))
 		return dir;
 
 	duplicated = xstrdup(name);
diff --git a/src/directory.h b/src/directory.h
index 9c3063d75..bb2888034 100644
--- a/src/directory.h
+++ b/src/directory.h
@@ -45,10 +45,10 @@ struct directory {
 
 extern struct directory music_root;
 
-static inline int isRootDirectory(const char *name)
+static inline int path_is_music_root(const char *name)
 {
-	/* TODO: verify and remove !name check */
-	return (!name || *name == '\0' || !strcmp(name, "/"));
+	assert(name);
+	return (!*name || !strcmp(name, "/"));
 }
 
 struct directory * directory_new(const char *dirname, struct directory *parent);
diff --git a/src/update.c b/src/update.c
index 7c2a7570a..72f4b943b 100644
--- a/src/update.c
+++ b/src/update.c
@@ -349,8 +349,8 @@ static void updatePath(const char *utf8path)
 
 static void * update_task(void *_path)
 {
-	if (_path != NULL && !isRootDirectory(_path)) {
-		updatePath((char *)_path);
+	if (_path && !path_is_music_root(_path)) {
+		updatePath(_path);
 		free(_path);
 	} else {
 		struct stat st;
-- 
cgit v1.2.3


From 3e652ee92d4bbe2a9e68cc80b6d58a7f0e6a6093 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 21:46:00 -0700
Subject: update: validate in command.c and fix small memory leak

If update_task was called with "" as its path argument, that
string would never get freed because it false positived.

Instead, never pass "" to update_task and trip an assertion
if we do.
---
 src/command.c | 12 +++++++++---
 src/update.c  | 11 ++++++++---
 2 files changed, 17 insertions(+), 6 deletions(-)

(limited to 'src')

diff --git a/src/command.c b/src/command.c
index f583f82a1..04e85c6fe 100644
--- a/src/command.c
+++ b/src/command.c
@@ -802,9 +802,15 @@ static int handleUpdate(int fd, mpd_unused int *permission,
 	char *path = NULL;
 
 	assert(argc <= 2);
-	if (argc == 2 && !(path = sanitizePathDup(argv[1]))) {
-		commandError(fd, ACK_ERROR_ARG, "invalid path");
-		return -1;
+	if (argc == 2) {
+		if (!(path = sanitizePathDup(argv[1]))) {
+			commandError(fd, ACK_ERROR_ARG, "invalid path");
+			return -1;
+		}
+		if (path_is_music_root(path)) {
+			free(path);
+			path = NULL;
+		}
 	}
 	return print_update_result(fd, directory_update_init(path));
 }
diff --git a/src/update.c b/src/update.c
index 72f4b943b..6c8b6e7bc 100644
--- a/src/update.c
+++ b/src/update.c
@@ -349,9 +349,12 @@ static void updatePath(const char *utf8path)
 
 static void * update_task(void *_path)
 {
-	if (_path && !path_is_music_root(_path)) {
-		updatePath(_path);
-		free(_path);
+	char *utf8path = _path;
+
+	if (utf8path) {
+		assert(*utf8path);
+		updatePath(utf8path);
+		free(utf8path);
 	} else {
 		struct stat st;
 
@@ -386,6 +389,8 @@ unsigned directory_update_init(char *path)
 {
 	assert(pthread_equal(pthread_self(), main_task));
 
+	assert(!path || (path && *path));
+
 	if (progress != UPDATE_PROGRESS_IDLE) {
 		unsigned next_task_id;
 
-- 
cgit v1.2.3


From afef3a6e68f4007f8e087b6ae359518ff59b04ef Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 21:49:29 -0700
Subject: update: allow music_root updates to be queued

Previously only updates with subdirectories being
specified could be queued.  No harm in queueing
full updates.
---
 src/update.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

(limited to 'src')

diff --git a/src/update.c b/src/update.c
index 6c8b6e7bc..35605ee51 100644
--- a/src/update.c
+++ b/src/update.c
@@ -394,10 +394,9 @@ unsigned directory_update_init(char *path)
 	if (progress != UPDATE_PROGRESS_IDLE) {
 		unsigned next_task_id;
 
-		if (!path)
-			return 0;
 		if (update_paths_nr == ARRAY_SIZE(update_paths)) {
-			free(path);
+			if (path)
+				free(path);
 			return 0;
 		}
 
-- 
cgit v1.2.3


From 7284123e3555159c6550d5ac57b8334bdd1af5c1 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Wed, 8 Oct 2008 01:49:39 -0700
Subject: songvec: avoid holding nr_lock during free(3)

We only need to lock sv->nr changes to prevent traversals ( why
it's called "nr_lock").  free(3) is a "slow" function on my
system; so we can avoid unnecessarily holding a lock long for
longer than needed.
---
 src/songvec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

(limited to 'src')

diff --git a/src/songvec.c b/src/songvec.c
index ce04a4373..3d512e917 100644
--- a/src/songvec.c
+++ b/src/songvec.c
@@ -48,10 +48,12 @@ int songvec_delete(struct songvec *sv, const struct mpd_song *del)
 	for (i = sv->nr; --i >= 0; ) {
 		if (sv->base[i] != del)
 			continue;
-		/* we _don't_ call freeSong() here */
+		/* we _don't_ call song_free() here */
 		if (!--sv->nr) {
+			pthread_mutex_unlock(&nr_lock);
 			free(sv->base);
 			sv->base = NULL;
+			return i;
 		} else {
 			memmove(&sv->base[i], &sv->base[i + 1],
 				(sv->nr - i + 1) * sizeof(struct mpd_song *));
@@ -76,12 +78,12 @@ void songvec_add(struct songvec *sv, struct mpd_song *add)
 void songvec_destroy(struct songvec *sv)
 {
 	pthread_mutex_lock(&nr_lock);
+	sv->nr = 0;
+	pthread_mutex_unlock(&nr_lock);
 	if (sv->base) {
 		free(sv->base);
 		sv->base = NULL;
 	}
-	sv->nr = 0;
-	pthread_mutex_unlock(&nr_lock);
 }
 
 int songvec_for_each(const struct songvec *sv,
-- 
cgit v1.2.3


From 4111d84ad5b72f871d35f76a47765c138a201611 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 22:03:05 -0700
Subject: dirvec: add dirvec_for_each iterator

This will make it easier to introduce locking
---
 src/dirvec.c | 16 ++++++++++++++++
 src/dirvec.h |  3 +++
 2 files changed, 19 insertions(+)

(limited to 'src')

diff --git a/src/dirvec.c b/src/dirvec.c
index fdfbb3434..73375fc70 100644
--- a/src/dirvec.c
+++ b/src/dirvec.c
@@ -68,3 +68,19 @@ void dirvec_destroy(struct dirvec *dv)
 	}
 	dv->nr = 0;
 }
+
+int dirvec_for_each(const struct dirvec *dv,
+                    int (*fn)(struct directory *, void *), void *arg)
+{
+	size_t i;
+
+	for (i = 0; i < dv->nr; ++i) {
+		struct directory *dir = dv->base[i];
+
+		assert(dir);
+		if (fn(dir, arg) < 0)
+			return -1;
+	}
+
+	return 0;
+}
diff --git a/src/dirvec.h b/src/dirvec.h
index 02496cd2b..b820e8739 100644
--- a/src/dirvec.h
+++ b/src/dirvec.h
@@ -23,4 +23,7 @@ static inline void dirvec_clear(struct dirvec *dv)
 
 void dirvec_destroy(struct dirvec *dv);
 
+int dirvec_for_each(const struct dirvec *dv,
+                    int (*fn)(struct directory *, void *), void *arg);
+
 #endif /* DIRVEC_H */
-- 
cgit v1.2.3


From e018b35b0d99d24e1f3779fd51c07154b370eca9 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 23:04:36 -0700
Subject: dirvec: use dirvec_for_each where it makes sense

This way we can introduce locking to allow safe traversals from
the main thread while we're updating.
---
 src/directory.c       | 60 +++++++++++++++++++++++++++++++--------------------
 src/directory_print.c | 22 +++++++++----------
 src/update.c          | 35 +++++++++++++++---------------
 3 files changed, 65 insertions(+), 52 deletions(-)

(limited to 'src')

diff --git a/src/directory.c b/src/directory.c
index e66349e02..6a0bd18fd 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -50,18 +50,17 @@ void directory_free(struct directory *dir)
 		free(dir);
 }
 
-void directory_prune_empty(struct directory *dir)
+static int dir_pruner(struct directory *dir, void *_dv)
 {
-	int i;
-	struct dirvec *dv = &dir->children;
+	directory_prune_empty(dir);
+	if (directory_is_empty(dir))
+		dirvec_delete((struct dirvec *)_dv, dir);
+	return 0;
+}
 
-	for (i = dv->nr; --i >= 0; ) {
-		directory_prune_empty(dv->base[i]);
-		if (directory_is_empty(dv->base[i]))
-			dirvec_delete(dv, dv->base[i]);
-	}
-	if (!dv->nr)
-		dirvec_destroy(dv);
+void directory_prune_empty(struct directory *dir)
+{
+	dirvec_for_each(&dir->children, dir_pruner, &dir->children);
 }
 
 struct directory *
@@ -97,27 +96,38 @@ directory_get_subdir(struct directory *dir, const char *name)
 	return found;
 }
 
-void directory_sort(struct directory *dir)
+static int directory_sort_x(struct directory *dir, mpd_unused void *arg)
 {
-	int i;
-	struct dirvec *dv = &dir->children;
+	directory_sort(dir);
+	return 0;
+}
 
-	dirvec_sort(dv);
+void directory_sort(struct directory *dir)
+{
+	dirvec_sort(&dir->children);
+	dirvec_for_each(&dir->children, directory_sort_x, NULL);
 	songvec_sort(&dir->songs);
+}
+
+struct dirwalk_arg {
+	int (*each_song) (struct mpd_song *, void *);
+	int (*each_dir) (struct directory *, void *);
+	void *data;
+};
 
-	for (i = dv->nr; --i >= 0; )
-		directory_sort(dv->base[i]);
+static int dirwalk_x(struct directory *dir, void *_arg)
+{
+	struct dirwalk_arg *arg = _arg;
+
+	return directory_walk(dir, arg->each_song, arg->each_dir, arg->data);
 }
 
-int
-directory_walk(struct directory *dir,
+int directory_walk(struct directory *dir,
 			  int (*forEachSong) (struct mpd_song *, void *),
 			  int (*forEachDir) (struct directory *, void *),
 			  void *data)
 {
-	struct dirvec *dv = &dir->children;
 	int err = 0;
-	size_t j;
 
 	if (forEachDir && (err = forEachDir(dir, data)) < 0)
 		return err;
@@ -128,9 +138,13 @@ directory_walk(struct directory *dir,
 			return err;
 	}
 
-	for (j = 0; err >= 0 && j < dv->nr; ++j)
-		err = directory_walk(dv->base[j], forEachSong,
-						forEachDir, data);
+	if (forEachDir) {
+		struct dirwalk_arg dw_arg;
 
+		dw_arg.each_song = forEachSong;
+		dw_arg.each_dir = forEachDir;
+		dw_arg.data = data;
+		err = dirvec_for_each(&dir->children, dirwalk_x, &dw_arg);
+	}
 	return err;
 }
diff --git a/src/directory_print.c b/src/directory_print.c
index 1c30f1608..d7b42ec59 100644
--- a/src/directory_print.c
+++ b/src/directory_print.c
@@ -23,25 +23,23 @@
 #include "songvec.h"
 #include "myfprintf.h"
 
-static int dirvec_print(int fd, const struct dirvec *dv)
+static int directory_print_info(struct directory *dir, int fd)
 {
-	size_t i;
-
-	for (i = 0; i < dv->nr; ++i) {
-		if (fdprintf(fd, DIRECTORY_DIR "%s\n",
-		             directory_get_path(dv->base[i])) < 0)
-			return -1;
-	}
+	return fdprintf(fd, DIRECTORY_DIR "%s\n", directory_get_path(dir));
+}
 
-	return 0;
+static int directory_print_info_x(struct directory *dir, void *data)
+{
+	return directory_print_info(dir, (int)(size_t)data);
 }
 
 int directory_print(int fd, const struct directory *dir)
 {
-	if (dirvec_print(fd, &dir->children) < 0)
+	void *arg = (void *)(size_t)fd;
+
+	if (dirvec_for_each(&dir->children, directory_print_info_x, arg) < 0)
 		return -1;
-	if (songvec_for_each(&dir->songs, song_print_info_x,
-	                     (void *)(size_t)fd) < 0)
+	if (songvec_for_each(&dir->songs, song_print_info_x, arg) < 0)
 		return -1;
 	return 0;
 }
diff --git a/src/update.c b/src/update.c
index 35605ee51..99a58a06e 100644
--- a/src/update.c
+++ b/src/update.c
@@ -91,15 +91,12 @@ static int delete_each_song(struct mpd_song *song, mpd_unused void *data)
  * Recursively remove all sub directories and songs from a directory,
  * leaving an empty directory.
  */
-static void clear_directory(struct directory *dir)
+static int clear_directory(struct directory *dir, mpd_unused void *arg)
 {
-	int i;
-
-	for (i = dir->children.nr; --i >= 0;)
-		clear_directory(dir->children.base[i]);
+	dirvec_for_each(&dir->children, clear_directory, NULL);
 	dirvec_clear(&dir->children);
-
 	songvec_for_each(&dir->songs, delete_each_song, dir);
+	return 0;
 }
 
 /**
@@ -109,7 +106,7 @@ static void delete_directory(struct directory *dir)
 {
 	assert(dir->parent != NULL);
 
-	clear_directory(dir);
+	clear_directory(dir, NULL);
 	dirvec_delete(&dir->parent->children, dir);
 	directory_free(dir);
 }
@@ -149,23 +146,27 @@ static void delete_path(const char *path)
 	}
 }
 
-static void
-removeDeletedFromDirectory(char *path_max_tmp, struct directory *dir)
+/* passed to dirvec_for_each */
+static int delete_directory_if_removed(struct directory *dir, void *_data)
 {
-	int i;
-	struct dirvec *dv = &dir->children;
-	struct delete_data data;
+	if (!isDir(dir->path)) {
+		struct delete_data *data = _data;
 
-	for (i = dv->nr; --i >= 0; ) {
-		if (isDir(dv->base[i]->path))
-			continue;
-		LOG("removing directory: %s\n", dv->base[i]->path);
-		dirvec_delete(dv, dv->base[i]);
+		LOG("removing directory: %s\n", directory_get_path(dir));
+		dirvec_delete(&data->dir->children, dir);
 		modified = 1;
 	}
+	return 0;
+}
+
+static void
+removeDeletedFromDirectory(char *path_max_tmp, struct directory *dir)
+{
+	struct delete_data data;
 
 	data.dir = dir;
 	data.tmp = path_max_tmp;
+	dirvec_for_each(&dir->children, delete_directory_if_removed, &data);
 	songvec_for_each(&dir->songs, delete_song_if_removed, &data);
 }
 
-- 
cgit v1.2.3


From ae8e81043dcf2169f4fb137a74df9926ed248f38 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 23:23:09 -0700
Subject: dirvec: introduce locking for all iterators

Like the songvec nr_lock, only one lock is used for all
traversals since they're rarely changed.  This only
projects traversals, but not the individual structures
themselves.
---
 src/dirvec.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

(limited to 'src')

diff --git a/src/dirvec.c b/src/dirvec.c
index 73375fc70..228c54be9 100644
--- a/src/dirvec.c
+++ b/src/dirvec.c
@@ -3,6 +3,8 @@
 #include "os_compat.h"
 #include "utils.h"
 
+static pthread_mutex_t nr_lock = PTHREAD_MUTEX_INITIALIZER;
+
 static size_t dv_size(struct dirvec *dv)
 {
 	return dv->nr * sizeof(struct directory *);
@@ -18,55 +20,71 @@ static int dirvec_cmp(const void *d1, const void *d2)
 
 void dirvec_sort(struct dirvec *dv)
 {
+	pthread_mutex_lock(&nr_lock);
 	qsort(dv->base, dv->nr, sizeof(struct directory *), dirvec_cmp);
+	pthread_mutex_unlock(&nr_lock);
 }
 
 struct directory *dirvec_find(const struct dirvec *dv, const char *path)
 {
 	int i;
+	struct directory *ret = NULL;
 
-	for (i = dv->nr; --i >= 0; )
-		if (!strcmp(dv->base[i]->path, path))
-			return dv->base[i];
-	return NULL;
+	pthread_mutex_lock(&nr_lock);
+	for (i = dv->nr; --i >= 0; ) {
+		if (strcmp(dv->base[i]->path, path))
+			continue;
+		ret = dv->base[i];
+		break;
+	}
+	pthread_mutex_unlock(&nr_lock);
+	return ret;
 }
 
 int dirvec_delete(struct dirvec *dv, struct directory *del)
 {
 	int i;
 
+	pthread_mutex_lock(&nr_lock);
 	for (i = dv->nr; --i >= 0; ) {
 		if (dv->base[i] != del)
 			continue;
 		/* we _don't_ call directory_free() here */
 		if (!--dv->nr) {
+			pthread_mutex_unlock(&nr_lock);
 			free(dv->base);
 			dv->base = NULL;
+			return i;
 		} else {
 			memmove(&dv->base[i], &dv->base[i + 1],
 				(dv->nr - i + 1) * sizeof(struct directory *));
 			dv->base = xrealloc(dv->base, dv_size(dv));
 		}
-		return i;
+		break;
 	}
+	pthread_mutex_unlock(&nr_lock);
 
-	return -1; /* not found */
+	return i;
 }
 
 void dirvec_add(struct dirvec *dv, struct directory *add)
 {
+	pthread_mutex_lock(&nr_lock);
 	++dv->nr;
 	dv->base = xrealloc(dv->base, dv_size(dv));
 	dv->base[dv->nr - 1] = add;
+	pthread_mutex_unlock(&nr_lock);
 }
 
 void dirvec_destroy(struct dirvec *dv)
 {
+	pthread_mutex_lock(&nr_lock);
+	dv->nr = 0;
+	pthread_mutex_unlock(&nr_lock);
 	if (dv->base) {
 		free(dv->base);
 		dv->base = NULL;
 	}
-	dv->nr = 0;
 }
 
 int dirvec_for_each(const struct dirvec *dv,
@@ -74,13 +92,17 @@ int dirvec_for_each(const struct dirvec *dv,
 {
 	size_t i;
 
+	pthread_mutex_lock(&nr_lock);
 	for (i = 0; i < dv->nr; ++i) {
 		struct directory *dir = dv->base[i];
 
 		assert(dir);
+		pthread_mutex_unlock(&nr_lock);
 		if (fn(dir, arg) < 0)
 			return -1;
+		pthread_mutex_lock(&nr_lock); /* dv->nr may change in fn() */
 	}
+	pthread_mutex_unlock(&nr_lock);
 
 	return 0;
 }
-- 
cgit v1.2.3


From f58827951ff79539125dcb4b1bd08e549e347a62 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 11 Oct 2008 23:59:45 -0700
Subject: update: serialize song_free in main thread

It's actually possible to have a traverser accessing a song
while we're freeing it in the main thread, as the songvec
iterators don't lock the individual elements.
---
 src/update.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

(limited to 'src')

diff --git a/src/update.c b/src/update.c
index 99a58a06e..c3e5b9a7c 100644
--- a/src/update.c
+++ b/src/update.c
@@ -74,9 +74,6 @@ static void delete_song(struct directory *dir, struct mpd_song *del)
 	wakeup_main_task();
 	do { cond_wait(&delete_cond); } while (delete);
 	cond_leave(&delete_cond);
-
-	/* finally, all possible references gone, free it */
-	song_free(del);
 }
 
 static int delete_each_song(struct mpd_song *song, mpd_unused void *data)
@@ -423,6 +420,7 @@ void reap_update_task(void)
 		char tmp[MPD_PATH_MAX];
 		LOG("removing: %s\n", song_get_url(delete, tmp));
 		deleteASongFromPlaylist(delete);
+		song_free(delete);
 		delete = NULL;
 		cond_signal(&delete_cond);
 	}
-- 
cgit v1.2.3


From 6ad5891f2c57bca861d51160cbfa54f172fa1ea5 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 12 Oct 2008 00:23:38 -0700
Subject: update: serialize directory deletions

We only delete directory object in the main thread to
prevent access to individual elements (mainly path).
---
 src/update.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

(limited to 'src')

diff --git a/src/update.c b/src/update.c
index c3e5b9a7c..b244c3a3b 100644
--- a/src/update.c
+++ b/src/update.c
@@ -46,7 +46,11 @@ static const unsigned update_task_id_max = 1 << 15;
 
 static unsigned update_task_id;
 
-static struct mpd_song *delete;
+enum update_type { UPDATE_TYPE_SONG, UPDATE_TYPE_DIR };
+
+static enum update_type delete_type;
+
+static void *delete;
 
 static struct condition delete_cond;
 
@@ -62,20 +66,26 @@ static void directory_set_stat(struct directory *dir, const struct stat *st)
 	dir->stat = 1;
 }
 
-static void delete_song(struct directory *dir, struct mpd_song *del)
+static void serialized_delete(void *del, enum update_type type)
 {
-	/* first, prevent traversers in main task from getting this */
-	songvec_delete(&dir->songs, del);
-
-	/* now take it out of the playlist (in the main_task) */
 	cond_enter(&delete_cond);
 	assert(!delete);
+	delete_type = type;
 	delete = del;
 	wakeup_main_task();
 	do { cond_wait(&delete_cond); } while (delete);
 	cond_leave(&delete_cond);
 }
 
+static void delete_song(struct directory *dir, struct mpd_song *del)
+{
+	/* first, prevent traversers in main task from getting this */
+	songvec_delete(&dir->songs, del);
+
+	/* now take it out of the playlist (in the main_task) and free */
+	serialized_delete(del, UPDATE_TYPE_SONG);
+}
+
 static int delete_each_song(struct mpd_song *song, mpd_unused void *data)
 {
 	struct directory *dir = data;
@@ -105,7 +115,7 @@ static void delete_directory(struct directory *dir)
 
 	clear_directory(dir, NULL);
 	dirvec_delete(&dir->parent->children, dir);
-	directory_free(dir);
+	serialized_delete(dir, UPDATE_TYPE_DIR);
 }
 
 struct delete_data {
@@ -417,10 +427,17 @@ void reap_update_task(void)
 
 	cond_enter(&delete_cond);
 	if (delete) {
-		char tmp[MPD_PATH_MAX];
-		LOG("removing: %s\n", song_get_url(delete, tmp));
-		deleteASongFromPlaylist(delete);
-		song_free(delete);
+		switch (delete_type) {
+		case UPDATE_TYPE_SONG: {
+			char tmp[MPD_PATH_MAX];
+			LOG("removing: %s\n", song_get_url(delete, tmp));
+			deleteASongFromPlaylist(delete);
+			song_free(delete);
+			}
+			break;
+		case UPDATE_TYPE_DIR:
+			directory_free(delete);
+		}
 		delete = NULL;
 		cond_signal(&delete_cond);
 	}
-- 
cgit v1.2.3


From a9f0147852c6dd4e355ac339fc4e3f2186cdb155 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 12 Oct 2008 01:47:11 -0700
Subject: directory: directory_free kills all that it contains

Only call this in the main thread; it is NOT thread
safe and not designed to be.
---
 src/directory.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

(limited to 'src')

diff --git a/src/directory.c b/src/directory.c
index 6a0bd18fd..709353476 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -42,8 +42,22 @@ struct directory * directory_new(const char *path, struct directory * parent)
 	return dir;
 }
 
+static int free_each_song(struct mpd_song *song, mpd_unused void *arg)
+{
+	song_free(song);
+	return 0;
+}
+
+static int free_each_dir(struct directory *dir, void *arg)
+{
+	if (arg != dir)
+		directory_free(dir);
+	return 0;
+}
+
 void directory_free(struct directory *dir)
 {
+	directory_walk(dir, free_each_song, free_each_dir, dir);
 	dirvec_destroy(&dir->children);
 	songvec_destroy(&dir->songs);
 	if (dir != &music_root)
-- 
cgit v1.2.3


From 085d139e21511109b0a38278baf20993c7edfa38 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 12 Oct 2008 01:56:13 -0700
Subject: update: remove delete_each_song and clear_directory

Our beefier directory_free takes care of it now
in the main task.
---
 src/update.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

(limited to 'src')

diff --git a/src/update.c b/src/update.c
index b244c3a3b..9c0113446 100644
--- a/src/update.c
+++ b/src/update.c
@@ -86,26 +86,6 @@ static void delete_song(struct directory *dir, struct mpd_song *del)
 	serialized_delete(del, UPDATE_TYPE_SONG);
 }
 
-static int delete_each_song(struct mpd_song *song, mpd_unused void *data)
-{
-	struct directory *dir = data;
-	assert(song->parent == dir);
-	delete_song(dir, song);
-	return 0;
-}
-
-/**
- * Recursively remove all sub directories and songs from a directory,
- * leaving an empty directory.
- */
-static int clear_directory(struct directory *dir, mpd_unused void *arg)
-{
-	dirvec_for_each(&dir->children, clear_directory, NULL);
-	dirvec_clear(&dir->children);
-	songvec_for_each(&dir->songs, delete_each_song, dir);
-	return 0;
-}
-
 /**
  * Recursively free a directory and all its contents.
  */
@@ -113,8 +93,10 @@ static void delete_directory(struct directory *dir)
 {
 	assert(dir->parent != NULL);
 
-	clear_directory(dir, NULL);
+	/* first, prevent traversers in main task from getting this */
 	dirvec_delete(&dir->parent->children, dir);
+
+	/* now we let the main task recursively delete everything under us */
 	serialized_delete(dir, UPDATE_TYPE_DIR);
 }
 
-- 
cgit v1.2.3


From d60f11020f4901b806c822ef5ab2d87bbb9e4077 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 12 Oct 2008 02:06:08 -0700
Subject: update: simplify the serialized_delete usage a bit

Pass a callback and argument to it instead of requiring
explicit type.
---
 src/update.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

(limited to 'src')

diff --git a/src/update.c b/src/update.c
index 9c0113446..9ede72759 100644
--- a/src/update.c
+++ b/src/update.c
@@ -46,9 +46,7 @@ static const unsigned update_task_id_max = 1 << 15;
 
 static unsigned update_task_id;
 
-enum update_type { UPDATE_TYPE_SONG, UPDATE_TYPE_DIR };
-
-static enum update_type delete_type;
+static void (*delete_fn)(void *);
 
 static void *delete;
 
@@ -66,24 +64,33 @@ static void directory_set_stat(struct directory *dir, const struct stat *st)
 	dir->stat = 1;
 }
 
-static void serialized_delete(void *del, enum update_type type)
+static void serialized_delete(void (*fn)(void *), void *del)
 {
 	cond_enter(&delete_cond);
 	assert(!delete);
-	delete_type = type;
+	assert(!delete_fn);
+	delete_fn = fn;
 	delete = del;
 	wakeup_main_task();
 	do { cond_wait(&delete_cond); } while (delete);
 	cond_leave(&delete_cond);
 }
 
+static void delete_song_safely(struct mpd_song *song)
+{
+	char tmp[MPD_PATH_MAX];
+	LOG("removing: %s\n", song_get_url(song, tmp));
+	deleteASongFromPlaylist(song);
+	song_free(song);
+}
+
 static void delete_song(struct directory *dir, struct mpd_song *del)
 {
 	/* first, prevent traversers in main task from getting this */
 	songvec_delete(&dir->songs, del);
 
 	/* now take it out of the playlist (in the main_task) and free */
-	serialized_delete(del, UPDATE_TYPE_SONG);
+	serialized_delete((void (*)(void *))delete_song_safely, del);
 }
 
 /**
@@ -97,7 +104,7 @@ static void delete_directory(struct directory *dir)
 	dirvec_delete(&dir->parent->children, dir);
 
 	/* now we let the main task recursively delete everything under us */
-	serialized_delete(dir, UPDATE_TYPE_DIR);
+	serialized_delete((void (*)(void *))directory_free, dir);
 }
 
 struct delete_data {
@@ -408,19 +415,9 @@ void reap_update_task(void)
 		return;
 
 	cond_enter(&delete_cond);
-	if (delete) {
-		switch (delete_type) {
-		case UPDATE_TYPE_SONG: {
-			char tmp[MPD_PATH_MAX];
-			LOG("removing: %s\n", song_get_url(delete, tmp));
-			deleteASongFromPlaylist(delete);
-			song_free(delete);
-			}
-			break;
-		case UPDATE_TYPE_DIR:
-			directory_free(delete);
-		}
-		delete = NULL;
+	if (delete && delete_fn) {
+		delete_fn(delete);
+		delete = delete_fn = NULL;
 		cond_signal(&delete_cond);
 	}
 	cond_leave(&delete_cond);
-- 
cgit v1.2.3


From bb106c3bf8daf366a4b4ca7229010c038f89755c Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 12 Oct 2008 03:35:45 -0700
Subject: directory: always maintain sorted properties vectors

This allows clients to see sorted results while we're
updating the DB and removes the need for us to have
to sort manually.

We'll have to write separate routines for managing stored
playlists with songvecs eventually; but that's for another day.
---
 src/database.c  |  4 ----
 src/directory.c | 13 -------------
 src/directory.h |  2 --
 src/dirvec.c    | 15 ++++++---------
 src/dirvec.h    |  2 --
 src/songvec.c   | 15 ++++++---------
 src/songvec.h   |  2 --
 7 files changed, 12 insertions(+), 41 deletions(-)

(limited to 'src')

diff --git a/src/database.c b/src/database.c
index a9e80f425..733180fea 100644
--- a/src/database.c
+++ b/src/database.c
@@ -183,10 +183,6 @@ int db_save(void)
 	DEBUG("removing empty directories from DB\n");
 	directory_prune_empty(&music_root);
 
-	DEBUG("sorting DB\n");
-
-	directory_sort(&music_root);
-
 	DEBUG("writing DB\n");
 
 	fd = open(dbFile, O_WRONLY|O_TRUNC|O_CREAT, 0666);
diff --git a/src/directory.c b/src/directory.c
index 709353476..3ebac6830 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -110,19 +110,6 @@ directory_get_subdir(struct directory *dir, const char *name)
 	return found;
 }
 
-static int directory_sort_x(struct directory *dir, mpd_unused void *arg)
-{
-	directory_sort(dir);
-	return 0;
-}
-
-void directory_sort(struct directory *dir)
-{
-	dirvec_sort(&dir->children);
-	dirvec_for_each(&dir->children, directory_sort_x, NULL);
-	songvec_sort(&dir->songs);
-}
-
 struct dirwalk_arg {
 	int (*each_song) (struct mpd_song *, void *);
 	int (*each_dir) (struct directory *, void *);
diff --git a/src/directory.h b/src/directory.h
index bb2888034..8f0d797a8 100644
--- a/src/directory.h
+++ b/src/directory.h
@@ -92,8 +92,6 @@ int directory_save(int fd, struct directory *dir);
 
 void directory_load(FILE *fp, struct directory *dir);
 
-void directory_sort(struct directory *dir);
-
 int db_walk(const char *name,
 		  int (*forEachSong) (struct mpd_song *, void *),
 		  int (*forEachDir) (struct directory *, void *), void *data);
diff --git a/src/dirvec.c b/src/dirvec.c
index 228c54be9..91b371b8a 100644
--- a/src/dirvec.c
+++ b/src/dirvec.c
@@ -18,13 +18,6 @@ static int dirvec_cmp(const void *d1, const void *d2)
 	return strcmp(a->path, b->path);
 }
 
-void dirvec_sort(struct dirvec *dv)
-{
-	pthread_mutex_lock(&nr_lock);
-	qsort(dv->base, dv->nr, sizeof(struct directory *), dirvec_cmp);
-	pthread_mutex_unlock(&nr_lock);
-}
-
 struct directory *dirvec_find(const struct dirvec *dv, const char *path)
 {
 	int i;
@@ -69,10 +62,14 @@ int dirvec_delete(struct dirvec *dv, struct directory *del)
 
 void dirvec_add(struct dirvec *dv, struct directory *add)
 {
+	size_t old_nr;
+
 	pthread_mutex_lock(&nr_lock);
-	++dv->nr;
+	old_nr = dv->nr++;
 	dv->base = xrealloc(dv->base, dv_size(dv));
-	dv->base[dv->nr - 1] = add;
+	dv->base[old_nr] = add;
+	if (old_nr && dirvec_cmp(&dv->base[old_nr - 1], &add) >= 0)
+		qsort(dv->base, dv->nr, sizeof(struct directory *), dirvec_cmp);
 	pthread_mutex_unlock(&nr_lock);
 }
 
diff --git a/src/dirvec.h b/src/dirvec.h
index b820e8739..6709537b8 100644
--- a/src/dirvec.h
+++ b/src/dirvec.h
@@ -8,8 +8,6 @@ struct dirvec {
 	size_t nr;
 };
 
-void dirvec_sort(struct dirvec *dv);
-
 struct directory *dirvec_find(const struct dirvec *dv, const char *path);
 
 int dirvec_delete(struct dirvec *dv, struct directory *del);
diff --git a/src/songvec.c b/src/songvec.c
index 3d512e917..1a9635044 100644
--- a/src/songvec.c
+++ b/src/songvec.c
@@ -17,13 +17,6 @@ static size_t sv_size(struct songvec *sv)
 	return sv->nr * sizeof(struct mpd_song *);
 }
 
-void songvec_sort(struct songvec *sv)
-{
-	pthread_mutex_lock(&nr_lock);
-	qsort(sv->base, sv->nr, sizeof(struct mpd_song *), songvec_cmp);
-	pthread_mutex_unlock(&nr_lock);
-}
-
 struct mpd_song *songvec_find(const struct songvec *sv, const char *url)
 {
 	int i;
@@ -68,10 +61,14 @@ int songvec_delete(struct songvec *sv, const struct mpd_song *del)
 
 void songvec_add(struct songvec *sv, struct mpd_song *add)
 {
+	size_t old_nr;
+
 	pthread_mutex_lock(&nr_lock);
-	++sv->nr;
+	old_nr = sv->nr++;
 	sv->base = xrealloc(sv->base, sv_size(sv));
-	sv->base[sv->nr - 1] = add;
+	sv->base[old_nr] = add;
+	if (old_nr && songvec_cmp(&sv->base[old_nr - 1], &add) >= 0)
+		qsort(sv->base, sv->nr, sizeof(struct mpd_song *), songvec_cmp);
 	pthread_mutex_unlock(&nr_lock);
 }
 
diff --git a/src/songvec.h b/src/songvec.h
index 633cf8d66..10a896052 100644
--- a/src/songvec.h
+++ b/src/songvec.h
@@ -9,8 +9,6 @@ struct songvec {
 	size_t nr;
 };
 
-void songvec_sort(struct songvec *sv);
-
 struct mpd_song *songvec_find(const struct songvec *sv, const char *url);
 
 int songvec_delete(struct songvec *sv, const struct mpd_song *del);
-- 
cgit v1.2.3


From c7fb996848715ee43f5df238aff75e561a4451b9 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 12 Oct 2008 04:48:31 -0700
Subject: directory: children leave parents before being free()ed

This prevents double-free()s when doing teardown of
a directory.
---
 src/directory.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

(limited to 'src')

diff --git a/src/directory.c b/src/directory.c
index 3ebac6830..40be57855 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -60,8 +60,11 @@ void directory_free(struct directory *dir)
 	directory_walk(dir, free_each_song, free_each_dir, dir);
 	dirvec_destroy(&dir->children);
 	songvec_destroy(&dir->songs);
-	if (dir != &music_root)
+	if (dir != &music_root) {
+		assert(dir->parent);
+		dirvec_delete(&dir->parent->children, dir);
 		free(dir);
+	}
 }
 
 static int dir_pruner(struct directory *dir, void *_dv)
-- 
cgit v1.2.3


From c7579ca2d8422f0172537e1ca7d1bd46edfc4f9d Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 12 Oct 2008 05:08:12 -0700
Subject: update: fix multiple deletes from *vec iterators

{song,dir}vec_for_each each failed to gracefully handle deleted
files when iterating through.  While we were thread-safe, we
were not safe within the calling thread.  If a callback we
passed caused sv->nr to shring, our index would still increment;
causing files to stay in the database.

A way to test this is to remove 10 or so contiguous songs from a
>10 song directory.
---
 src/dirvec.c  | 6 +++++-
 src/songvec.c | 6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

(limited to 'src')

diff --git a/src/dirvec.c b/src/dirvec.c
index 91b371b8a..a5eb6d54e 100644
--- a/src/dirvec.c
+++ b/src/dirvec.c
@@ -88,16 +88,20 @@ int dirvec_for_each(const struct dirvec *dv,
                     int (*fn)(struct directory *, void *), void *arg)
 {
 	size_t i;
+	size_t prev_nr;
 
 	pthread_mutex_lock(&nr_lock);
-	for (i = 0; i < dv->nr; ++i) {
+	for (i = 0; i < dv->nr; ) {
 		struct directory *dir = dv->base[i];
 
 		assert(dir);
+		prev_nr = dv->nr;
 		pthread_mutex_unlock(&nr_lock);
 		if (fn(dir, arg) < 0)
 			return -1;
 		pthread_mutex_lock(&nr_lock); /* dv->nr may change in fn() */
+		if (prev_nr == dv->nr)
+			++i;
 	}
 	pthread_mutex_unlock(&nr_lock);
 
diff --git a/src/songvec.c b/src/songvec.c
index 1a9635044..19433a222 100644
--- a/src/songvec.c
+++ b/src/songvec.c
@@ -87,18 +87,22 @@ int songvec_for_each(const struct songvec *sv,
                      int (*fn)(struct mpd_song *, void *), void *arg)
 {
 	size_t i;
+	size_t prev_nr;
 
 	pthread_mutex_lock(&nr_lock);
-	for (i = 0; i < sv->nr; ++i) {
+	for (i = 0; i < sv->nr; ) {
 		struct mpd_song *song = sv->base[i];
 
 		assert(song);
 		assert(*song->url);
 
+		prev_nr = sv->nr;
 		pthread_mutex_unlock(&nr_lock); /* fn() may block */
 		if (fn(song, arg) < 0)
 			return -1;
 		pthread_mutex_lock(&nr_lock); /* sv->nr may change in fn() */
+		if (prev_nr == sv->nr)
+			++i;
 	}
 	pthread_mutex_unlock(&nr_lock);
 
-- 
cgit v1.2.3