directory: simplify list update handling logic

Now the "update" command can be issued multiple times regardless
of whether the client is in list mode or not.

We serialize the update tasks to prevent updates from trampling
over each other and will spawn another update task
once the current one is finished updating and reaped.

Right now we cap the queue size to 32 which is probably enough (I
bet most people usually run update with no argument anyways);
but we can make it grow/shrink dynamically if needed.  There'll
still be a hard-coded limit to prevent DoS attacks, though.

3 files changed, 71 insertions, 100 deletions

diff --git a/src/command.c b/src/command.c
index 70bfa78b5..07f422720 100644
--- a/src/command.c
+++ b/src/command.c
@@ -813,47 +813,17 @@ static int print_update_result(int fd, int ret)
 	return -1;
 }
 
-static int listHandleUpdate(int fd,
-			    mpd_unused int *permission,
-			    mpd_unused int argc,
-			    char *argv[],
-			    struct strnode *cmdnode, CommandEntry * cmd)
-{
-	static char **pathv;
-	static int pathc;
-	CommandEntry *nextCmd = NULL;
-	struct strnode *next = cmdnode->next;
-	int last = pathc++;
-
-	pathv = xrealloc(pathv, pathc * sizeof(char *));
-	pathv[last] = sanitizePathDup(argc == 2 ? argv[1] : "");
-
-	if (next)
-		nextCmd = getCommandEntryFromString(next->data, permission);
-
-	if (cmd != nextCmd) {
-		int ret = print_update_result(fd, updateInit(pathc, pathv));
-		if (pathc) {
-			assert(pathv);
-			free(pathv);
-			pathv = NULL;
-			pathc = 0;
-		}
-		return ret;
-	}
-
-	return 0;
-}
-
 static int handleUpdate(int fd, mpd_unused int *permission,
 			mpd_unused int argc, char *argv[])
 {
-	char *pathv[1];
+	char *path = NULL;
 
 	assert(argc <= 2);
-	if (argc == 2)
-		pathv[0] = sanitizePathDup(argv[1]);
-	return print_update_result(fd, updateInit(argc - 1, pathv));
+	if (argc == 2 && !(path = sanitizePathDup(argv[1]))) {
+		commandError(fd, ACK_ERROR_ARG, "invalid path");
+		return -1;
+	}
+	return print_update_result(fd, directory_update_init(path));
 }
 
 static int handleNext(mpd_unused int fd, mpd_unused int *permission,
@@ -1289,7 +1259,7 @@ void initCommands(void)
 	addCommand(COMMAND_PLAYLISTINFO,     PERMISSION_READ,    0,   1,   handlePlaylistInfo,         NULL);
 	addCommand(COMMAND_FIND,             PERMISSION_READ,    2,   -1,  handleFind,                 NULL);
 	addCommand(COMMAND_SEARCH,           PERMISSION_READ,    2,   -1,  handleSearch,               NULL);
-	addCommand(COMMAND_UPDATE,           PERMISSION_ADMIN,   0,   1,   handleUpdate,               listHandleUpdate);
+	addCommand(COMMAND_UPDATE,           PERMISSION_ADMIN,   0,   1,   handleUpdate,               NULL);
 	addCommand(COMMAND_NEXT,             PERMISSION_CONTROL, 0,   0,   handleNext,                 NULL);
 	addCommand(COMMAND_PREVIOUS,         PERMISSION_CONTROL, 0,   0,   handlePrevious,             NULL);
 	addCommand(COMMAND_LISTALL,          PERMISSION_READ,    0,   1,   handleListAll,              NULL);
diff --git a/src/directory.c b/src/directory.c
index 0c34e2ea9..1cd9da9eb 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -52,13 +52,18 @@ enum update_progress {
 	UPDATE_PROGRESS_DONE = 2
 } progress;
 
+/* make this dynamic?, or maybe this is big enough... */
+static char *update_paths[32];
+static size_t update_paths_nr;
+
 static Directory *music_root;
 
 static time_t directory_dbModTime;
 
 static pthread_t update_thr;
 
-static int directory_updateJobId;
+static const int update_task_id_max = 1 << 15;
+static int update_task_id;
 
 static int addToDirectory(Directory * directory, const char *name);
 
@@ -97,87 +102,84 @@ static char *getDbFile(void)
 
 int isUpdatingDB(void)
 {
-	return (progress != UPDATE_PROGRESS_IDLE) ? directory_updateJobId : 0;
-}
-
-void reap_update_task(void)
-{
-	enum update_return ret;
-
-	if (progress != UPDATE_PROGRESS_DONE)
-		return;
-	if (pthread_join(update_thr, (void **)&ret))
-		FATAL("error joining update thread: %s\n", strerror(errno));
-	if (ret == UPDATE_RETURN_UPDATED)
-		playlistVersionChange();
-	progress = UPDATE_PROGRESS_IDLE;
+	return (progress != UPDATE_PROGRESS_IDLE) ? update_task_id : 0;
 }
 
-/* @argv represents a null-terminated array of (null-terminated) strings */
-static void * update_task(void *argv)
+static void * update_task(void *_path)
 {
 	enum update_return ret = UPDATE_RETURN_NOUPDATE;
 
-	if (argv) {
-		char **pathv;
-		for (pathv = (char **)argv; *pathv; pathv++) {
-			switch (updatePath(*pathv)) {
-			case UPDATE_RETURN_ERROR:
-				ret = UPDATE_RETURN_ERROR;
-				goto out;
-			case UPDATE_RETURN_NOUPDATE:
-				break;
-			case UPDATE_RETURN_UPDATED:
-				ret = UPDATE_RETURN_UPDATED;
-			}
-			free(*pathv);
-		}
-		free(argv);
+	if (_path) {
+		ret = updatePath((char *)_path);
+		free(_path);
 	} else {
 		ret = updateDirectory(music_root);
 	}
 
 	if (ret == UPDATE_RETURN_UPDATED && writeDirectoryDB() < 0)
 		ret = UPDATE_RETURN_ERROR;
-out:
 	progress = UPDATE_PROGRESS_DONE;
 	wakeup_main_task();
 	return (void *)ret;
 }
 
-int updateInit(int argc, char *argv[])
+static void spawn_update_task(char *path)
 {
 	pthread_attr_t attr;
-	char **pathv = NULL;
-	int i;
-
-	if (progress != UPDATE_PROGRESS_IDLE) {
-		for (i = argc; --i >= 0; )
-			free(argv[i]);
-		return -1;
-	}
 
-	for (i = argc; --i >= 0; ) {
-		if (!argv[i])
-			return -2;
-	}
+	assert(pthread_equal(pthread_self(), main_task));
 
 	progress = UPDATE_PROGRESS_RUNNING;
-	if (argc > 0) {
-		pathv = xmalloc((argc + 1) * sizeof(char *));
-		memcpy(pathv, argv, argc * sizeof(char *));
-		pathv[argc] = NULL;
-	}
-
 	pthread_attr_init(&attr);
-	if (pthread_create(&update_thr, &attr, update_task, pathv))
+	if (pthread_create(&update_thr, &attr, update_task, path))
 		FATAL("Failed to spawn update task: %s\n", strerror(errno));
-	directory_updateJobId++;
-	if (directory_updateJobId > 1 << 15)
-		directory_updateJobId = 1;
-	DEBUG("updateInit: spawned update thread for update job id %i\n",
-	      (int)directory_updateJobId);
-	return directory_updateJobId;
+	if (++update_task_id > update_task_id_max)
+		update_task_id = 1;
+	DEBUG("spawned thread for update job id %i\n", update_task_id);
+}
+
+void reap_update_task(void)
+{
+	enum update_return ret;
+
+	assert(pthread_equal(pthread_self(), main_task));
+
+	if (progress != UPDATE_PROGRESS_DONE)
+		return;
+	if (pthread_join(update_thr, (void **)&ret))
+		FATAL("error joining update thread: %s\n", strerror(errno));
+	if (ret == UPDATE_RETURN_UPDATED)
+		playlistVersionChange();
+
+	if (update_paths_nr) {
+		char *path = update_paths[0];
+		memmove(&update_paths[0], &update_paths[1],
+		        --update_paths_nr * sizeof(char *));
+		spawn_update_task(path);
+	} else {
+		progress = UPDATE_PROGRESS_IDLE;
+	}
+}
+
+int directory_update_init(char *path)
+{
+	assert(pthread_equal(pthread_self(), main_task));
+
+	if (progress != UPDATE_PROGRESS_IDLE) {
+		int next_task_id;
+
+		if (!path)
+			return -1;
+		if (update_paths_nr == ARRAY_SIZE(update_paths))
+			return -1;
+		assert(update_paths_nr < ARRAY_SIZE(update_paths));
+		update_paths[update_paths_nr++] = path;
+		next_task_id = update_task_id + update_paths_nr;
+
+		return next_task_id > update_task_id_max ?  1 : next_task_id;
+	}
+	spawn_update_task(path);
+	return update_task_id;
 }
 
 static void directory_set_stat(Directory * dir, const struct stat *st)
diff --git a/src/directory.h b/src/directory.h
index eb1b35aa7..20b784166 100644
--- a/src/directory.h
+++ b/src/directory.h
@@ -43,11 +43,10 @@ int isUpdatingDB(void);
 
 /*
  * returns the non-negative update job ID on success,
- * -1 if busy, -2 if invalid argument
- * @argv itself is safe to free once updateInit returns, but the
- * string values contained by @argv MUST NOT be freed manually
+ * returns -1 if busy
+ * @path will be freed by this function and should not be reused
  */
-int updateInit(int argc, char *argv[]);
+int directory_update_init(char *path);
 
 void directory_init(void);