From e3cd520c6441f91655e799e60fe49c4bb7de13bc Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 22 Sep 2008 01:14:07 -0700
Subject: directory: use enum update_return for return values
This way we avoid having to document -1, 0, 1
---
src/directory.c | 121 +++++++++++++++++++++++---------------------------------
1 file changed, 49 insertions(+), 72 deletions(-)
diff --git a/src/directory.c b/src/directory.c
index 230dea5dc..08b3fb0e2 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -48,9 +48,11 @@
#define DIRECTORY_UPDATE_EXIT_UPDATE 1
#define DIRECTORY_UPDATE_EXIT_ERROR 2
-#define DIRECTORY_RETURN_NOUPDATE 0
-#define DIRECTORY_RETURN_UPDATE 1
-#define DIRECTORY_RETURN_ERROR -1
+enum update_return {
+ UPDATE_RETURN_ERROR = -1,
+ UPDATE_RETURN_NOUPDATE = 0,
+ UPDATE_RETURN_UPDATED = 1
+};
static Directory *mp3rootDirectory;
@@ -73,16 +75,16 @@ static void freeDirectoryList(DirectoryList * list);
static void freeDirectory(Directory * directory);
-static int exploreDirectory(Directory * directory);
+static enum update_return exploreDirectory(Directory * directory);
-static int updateDirectory(Directory * directory);
+static enum update_return updateDirectory(Directory * directory);
static void deleteEmptyDirectoriesInDirectory(Directory * directory);
static void removeSongFromDirectory(Directory * directory,
const char *shortname);
-static int addSubDirectoryToDirectory(Directory * directory,
+static enum update_return addSubDirectoryToDirectory(Directory * directory,
const char *shortname,
const char *name, struct stat *st);
@@ -94,7 +96,7 @@ static Directory *getDirectory(const char *name);
static Song *getSongDetails(const char *file, const char **shortnameRet,
Directory ** directoryRet);
-static int updatePath(const char *utf8path);
+static enum update_return updatePath(const char *utf8path);
static void sortDirectory(Directory * directory);
@@ -175,7 +177,7 @@ int updateInit(int fd, List * pathList)
directory_updatePid = fork();
if (directory_updatePid == 0) {
/* child */
- int dbUpdated = 0;
+ enum update_return dbUpdated = UPDATE_RETURN_NOUPDATE;
unblockSignals();
@@ -198,7 +200,7 @@ int updateInit(int fd, List * pathList)
while (node) {
switch (updatePath(node->key)) {
case 1:
- dbUpdated = 1;
+ dbUpdated = UPDATE_RETURN_UPDATED;
break;
case 0:
break;
@@ -208,12 +210,11 @@ int updateInit(int fd, List * pathList)
node = node->nextNode;
}
} else {
- if ((dbUpdated = updateDirectory(mp3rootDirectory)) < 0) {
+ if ((dbUpdated = updateDirectory(mp3rootDirectory)) < 0)
exit(DIRECTORY_UPDATE_EXIT_ERROR);
- }
}
- if (!dbUpdated)
+ if (dbUpdated == UPDATE_RETURN_NOUPDATE)
exit(DIRECTORY_UPDATE_EXIT_NOUPDATE);
/* ignore signals since we don't want them to corrupt the db */
@@ -320,12 +321,7 @@ static void deleteEmptyDirectoriesInDirectory(Directory * directory)
}
}
-/* return values:
- -1 -> error
- 0 -> no error, but nothing updated
- 1 -> no error, and stuff updated
- */
-static int updateInDirectory(Directory * directory,
+static enum update_return updateInDirectory(Directory * directory,
const char *shortname, const char *name)
{
Song *song;
@@ -333,17 +329,17 @@ static int updateInDirectory(Directory * directory,
struct stat st;
if (myStat(name, &st))
- return -1;
+ return UPDATE_RETURN_ERROR;
if (S_ISREG(st.st_mode) && hasMusicSuffix(name, 0)) {
if (!(song = songvec_find(&directory->songs, shortname))) {
addToDirectory(directory, shortname, name);
- return DIRECTORY_RETURN_UPDATE;
+ return UPDATE_RETURN_UPDATED;
} else if (st.st_mtime != song->mtime) {
LOG("updating %s\n", name);
if (updateSongInfo(song) < 0)
removeSongFromDirectory(directory, shortname);
- return 1;
+ return UPDATE_RETURN_UPDATED;
}
} else if (S_ISDIR(st.st_mode)) {
if (findInList
@@ -356,7 +352,7 @@ static int updateInDirectory(Directory * directory,
}
}
- return 0;
+ return UPDATE_RETURN_NOUPDATE;
}
/* we don't look at hidden files nor files with newlines in them */
@@ -365,18 +361,14 @@ static int skip_path(const char *path)
return (path[0] == '.' || strchr(path, '\n')) ? 1 : 0;
}
-/* return values:
- -1 -> error
- 0 -> no error, but nothing removed
- 1 -> no error, and stuff removed
- */
-static int removeDeletedFromDirectory(char *path_max_tmp, Directory * directory)
+static enum update_return
+removeDeletedFromDirectory(char *path_max_tmp, Directory * directory)
{
const char *dirname = (directory && directory->path) ?
directory->path : NULL;
ListNode *node, *tmpNode;
DirectoryList *subdirs = directory->subDirectories;
- int ret = 0;
+ enum update_return ret = UPDATE_RETURN_NOUPDATE;
int i;
struct songvec *sv = &directory->songs;
@@ -393,7 +385,7 @@ static int removeDeletedFromDirectory(char *path_max_tmp, Directory * directory)
if (!isDir(path_max_tmp)) {
LOG("removing directory: %s\n", path_max_tmp);
deleteFromList(subdirs, node->key);
- ret = 1;
+ ret = UPDATE_RETURN_UPDATED;
}
}
node = tmpNode;
@@ -411,7 +403,7 @@ static int removeDeletedFromDirectory(char *path_max_tmp, Directory * directory)
if (!isFile(path_max_tmp, NULL)) {
removeSongFromDirectory(directory, song->url);
- ret = 1;
+ ret = UPDATE_RETURN_UPDATED;
}
}
@@ -483,12 +475,7 @@ static Directory *addParentPathToDB(const char *utf8path, const char **shortname
return (Directory *) parentDirectory;
}
-/* return values:
- -1 -> error
- 0 -> no error, but nothing updated
- 1 -> no error, and stuff updated
- */
-static int updatePath(const char *utf8path)
+static enum update_return updatePath(const char *utf8path)
{
Directory *directory;
Directory *parentDirectory;
@@ -496,11 +483,11 @@ static int updatePath(const char *utf8path)
const char *shortname;
char *path = sanitizePathDup(utf8path);
time_t mtime;
- int ret = 0;
+ enum update_return ret = UPDATE_RETURN_NOUPDATE;
char path_max_tmp[MPD_PATH_MAX];
if (NULL == path)
- return -1;
+ return UPDATE_RETURN_ERROR;
/* if path is in the DB try to update it, or else delete it */
if ((directory = getDirectoryDetails(path, &shortname))) {
@@ -515,21 +502,21 @@ static int updatePath(const char *utf8path)
/* we don't want to delete the root directory */
else if (directory == mp3rootDirectory) {
free(path);
- return 0;
+ return UPDATE_RETURN_NOUPDATE;
}
/* if updateDirectory fails, means we should delete it */
else {
LOG("removing directory: %s\n", path);
deleteFromList(parentDirectory->subDirectories,
shortname);
- ret = 1;
+ ret = UPDATE_RETURN_UPDATED;
/* don't return, path maybe a song now */
}
} else if ((song = getSongDetails(path, &shortname, &parentDirectory))) {
if (!parentDirectory->stat
&& statDirectory(parentDirectory) < 0) {
free(path);
- return 0;
+ return UPDATE_RETURN_NOUPDATE;
}
/* if this song update is successfull, we are done */
else if (0 == inodeFoundInParent(parentDirectory->parent,
@@ -539,19 +526,19 @@ static int updatePath(const char *utf8path)
isMusic(get_song_url(path_max_tmp, song), &mtime, 0)) {
free(path);
if (song->mtime == mtime)
- return 0;
+ return UPDATE_RETURN_NOUPDATE;
else if (updateSongInfo(song) == 0)
- return 1;
+ return UPDATE_RETURN_UPDATED;
else {
removeSongFromDirectory(parentDirectory,
shortname);
- return 1;
+ return UPDATE_RETURN_UPDATED;
}
}
/* if updateDirectory fails, means we should delete it */
else {
removeSongFromDirectory(parentDirectory, shortname);
- ret = 1;
+ ret = UPDATE_RETURN_UPDATED;
/* don't return, path maybe a directory now */
}
}
@@ -569,7 +556,7 @@ static int updatePath(const char *utf8path)
parentDirectory->device)
&& addToDirectory(parentDirectory, shortname, path)
> 0) {
- ret = 1;
+ ret = UPDATE_RETURN_UPDATED;
}
}
@@ -586,32 +573,27 @@ static const char *opendir_path(char *path_max_tmp, const char *dirname)
return musicDir;
}
-/* return values:
- -1 -> error
- 0 -> no error, but nothing updated
- 1 -> no error, and stuff updated
- */
-static int updateDirectory(Directory * directory)
+static enum update_return updateDirectory(Directory * directory)
{
DIR *dir;
const char *dirname = getDirectoryPath(directory);
struct dirent *ent;
char path_max_tmp[MPD_PATH_MAX];
- int ret = 0;
+ enum update_return ret = UPDATE_RETURN_NOUPDATE;
if (!directory->stat && statDirectory(directory) < 0)
- return -1;
+ return UPDATE_RETURN_ERROR;
else if (inodeFoundInParent(directory->parent,
directory->inode,
directory->device))
- return -1;
+ return UPDATE_RETURN_ERROR;
dir = opendir(opendir_path(path_max_tmp, dirname));
if (!dir)
- return -1;
+ return UPDATE_RETURN_ERROR;
if (removeDeletedFromDirectory(path_max_tmp, directory) > 0)
- ret = 1;
+ ret = UPDATE_RETURN_UPDATED;
while ((ent = readdir(dir))) {
char *utf8;
@@ -626,7 +608,7 @@ static int updateDirectory(Directory * directory)
utf8 = pfx_dir(path_max_tmp, utf8, strlen(utf8),
dirname, strlen(dirname));
if (updateInDirectory(directory, utf8, path_max_tmp) > 0)
- ret = 1;
+ ret = UPDATE_RETURN_UPDATED;
}
closedir(dir);
@@ -634,24 +616,19 @@ static int updateDirectory(Directory * directory)
return ret;
}
-/* return values:
- -1 -> error
- 0 -> no error, but nothing found
- 1 -> no error, and stuff found
- */
-static int exploreDirectory(Directory * directory)
+static enum update_return exploreDirectory(Directory * directory)
{
DIR *dir;
const char *dirname = getDirectoryPath(directory);
struct dirent *ent;
char path_max_tmp[MPD_PATH_MAX];
- int ret = 0;
+ enum update_return ret = UPDATE_RETURN_NOUPDATE;
DEBUG("explore: attempting to opendir: %s\n", dirname);
dir = opendir(opendir_path(path_max_tmp, dirname));
if (!dir)
- return -1;
+ return UPDATE_RETURN_ERROR;
DEBUG("explore: %s\n", dirname);
@@ -670,7 +647,7 @@ static int exploreDirectory(Directory * directory)
utf8 = pfx_dir(path_max_tmp, utf8, strlen(utf8),
dirname, strlen(dirname));
if (addToDirectory(directory, utf8, path_max_tmp) > 0)
- ret = 1;
+ ret = UPDATE_RETURN_UPDATED;
}
closedir(dir);
@@ -705,26 +682,26 @@ static int inodeFoundInParent(Directory * parent, ino_t inode, dev_t device)
return 0;
}
-static int addSubDirectoryToDirectory(Directory * directory,
+static enum update_return addSubDirectoryToDirectory(Directory * directory,
const char *shortname,
const char *name, struct stat *st)
{
Directory *subDirectory;
if (inodeFoundInParent(directory, st->st_ino, st->st_dev))
- return 0;
+ return UPDATE_RETURN_NOUPDATE;
subDirectory = newDirectory(name, directory);
directory_set_stat(subDirectory, st);
if (exploreDirectory(subDirectory) < 1) {
freeDirectory(subDirectory);
- return 0;
+ return UPDATE_RETURN_NOUPDATE;
}
insertInList(directory->subDirectories, shortname, subDirectory);
- return 1;
+ return UPDATE_RETURN_UPDATED;
}
static int addToDirectory(Directory * directory,
--
cgit v1.2.3
From ada24f9a921ff95d874195acf253b5a9dd12213d Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 22 Sep 2008 02:04:23 -0700
Subject: directory: update do its work inside a thread
A lot of the preparation was needed (and done in previous
months) in making update thread-safe, but here it is.
This was the first thing I made work inside a thread when I
started mpd-uclinux many years ago, and also the last thing I've
done in mainline mpd to work inside a thread, go figure.
---
src/command.c | 18 ++----
src/directory.c | 158 +++++++++++++++++------------------------------------
src/directory.h | 2 +-
src/main.c | 2 +-
src/sig_handlers.c | 1 -
5 files changed, 56 insertions(+), 125 deletions(-)
diff --git a/src/command.c b/src/command.c
index ebca41bde..f5996c576 100644
--- a/src/command.c
+++ b/src/command.c
@@ -805,13 +805,10 @@ static int listHandleUpdate(int fd,
char *argv[],
struct strnode *cmdnode, CommandEntry * cmd)
{
- static List *pathList;
+ List *pathList = makeList(NULL, 1);
CommandEntry *nextCmd = NULL;
struct strnode *next = cmdnode->next;
- if (!pathList)
- pathList = makeList(NULL, 1);
-
if (argc == 2)
insertInList(pathList, argv[1], NULL);
else
@@ -820,12 +817,8 @@ static int listHandleUpdate(int fd,
if (next)
nextCmd = getCommandEntryFromString(next->data, permission);
- if (cmd != nextCmd) {
- int ret = updateInit(fd, pathList);
- freeList(pathList);
- pathList = NULL;
- return ret;
- }
+ if (cmd != nextCmd)
+ return updateInit(fd, pathList);
return 0;
}
@@ -834,12 +827,9 @@ static int handleUpdate(int fd, mpd_unused int *permission,
mpd_unused int argc, char *argv[])
{
if (argc == 2) {
- int ret;
List *pathList = makeList(NULL, 1);
insertInList(pathList, argv[1], NULL);
- ret = updateInit(fd, pathList);
- freeList(pathList);
- return ret;
+ return updateInit(fd, pathList);
}
return updateInit(fd, NULL);
}
diff --git a/src/directory.c b/src/directory.c
index 08b3fb0e2..cb1995a93 100644
--- a/src/directory.c
+++ b/src/directory.c
@@ -54,17 +54,19 @@ enum update_return {
UPDATE_RETURN_UPDATED = 1
};
+enum update_progress {
+ UPDATE_PROGRESS_IDLE = 0,
+ UPDATE_PROGRESS_RUNNING = 1,
+ UPDATE_PROGRESS_DONE = 2
+} progress;
+
static Directory *mp3rootDirectory;
static time_t directory_dbModTime;
-static sig_atomic_t directory_updatePid;
-
-static sig_atomic_t update_exited;
-
-static sig_atomic_t update_status;
+static pthread_t update_thr;
-static sig_atomic_t directory_updateJobId;
+static int directory_updateJobId;
static DirectoryList *newDirectoryList(void);
@@ -116,127 +118,67 @@ static char *getDbFile(void)
int isUpdatingDB(void)
{
- return directory_updatePid > 0 ? directory_updateJobId : 0;
+ return (progress != UPDATE_PROGRESS_IDLE) ? directory_updateJobId : 0;
}
-void directory_sigChldHandler(int pid, int status)
+void reap_update_task(void)
{
- if (directory_updatePid == pid) {
- update_status = status;
- update_exited = 1;
- wakeup_main_task();
- }
+ if (progress != UPDATE_PROGRESS_DONE)
+ return;
+ pthread_join(update_thr, NULL);
+ progress = UPDATE_PROGRESS_IDLE;
}
-void readDirectoryDBIfUpdateIsFinished(void)
+static void * update_task(void *arg)
{
- int status;
-
- if (!update_exited)
- return;
+ List *path_list = (List *)arg;
+ enum update_return ret = UPDATE_RETURN_NOUPDATE;
- status = update_status;
-
- if (WIFSIGNALED(status) && WTERMSIG(status) != SIGTERM) {
- ERROR("update process died from a non-TERM signal: %d\n",
- WTERMSIG(status));
- } else if (!WIFSIGNALED(status)) {
- switch (WEXITSTATUS(status)) {
- case DIRECTORY_UPDATE_EXIT_UPDATE:
- DEBUG("update finished successfully with changes\n");
- readDirectoryDB();
- DEBUG("update changes read into memory\n");
- playlistVersionChange();
- case DIRECTORY_UPDATE_EXIT_NOUPDATE:
- DEBUG("update exited successfully with no changes\n");
- break;
- default:
- ERROR("error updating db\n");
+ if (path_list) {
+ ListNode *node = path_list->firstNode;
+
+ while (node) {
+ switch (updatePath(node->key)) {
+ case UPDATE_RETURN_ERROR:
+ ret = UPDATE_RETURN_ERROR;
+ goto out;
+ case UPDATE_RETURN_NOUPDATE:
+ break;
+ case UPDATE_RETURN_UPDATED:
+ ret = UPDATE_RETURN_UPDATED;
+ }
+ node = node->nextNode;
}
+ free(path_list);
+ } else {
+ ret = updateDirectory(mp3rootDirectory);
}
- update_exited = 0;
- directory_updatePid = 0;
+
+ 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 fd, List * pathList)
+int updateInit(int fd, List * path_list)
{
- if (directory_updatePid > 0) {
- commandError(fd, ACK_ERROR_UPDATE_ALREADY, "already updating");
- return -1;
- }
-
- /*
- * need to block CHLD signal, cause it can exit before we
- * even get a chance to assign directory_updatePID
- *
- * Update: our signal blocking is is utterly broken by
- * pthreads(); goal will be to remove dependency on signals;
- * but for now use the my_usleep hack below.
- */
- blockSignals();
- directory_updatePid = fork();
- if (directory_updatePid == 0) {
- /* child */
- enum update_return dbUpdated = UPDATE_RETURN_NOUPDATE;
-
- unblockSignals();
-
- finishSigHandlers();
- closeAllListenSockets();
- client_manager_deinit();
- finishPlaylist();
- finishVolume();
-
- /*
- * XXX HACK to workaround race condition where
- * directory_updatePid is still zero in the parent even upon
- * entry of directory_sigChldHandler.
- */
- my_usleep(100000);
-
- if (pathList) {
- ListNode *node = pathList->firstNode;
-
- while (node) {
- switch (updatePath(node->key)) {
- case 1:
- dbUpdated = UPDATE_RETURN_UPDATED;
- break;
- case 0:
- break;
- default:
- exit(DIRECTORY_UPDATE_EXIT_ERROR);
- }
- node = node->nextNode;
- }
- } else {
- if ((dbUpdated = updateDirectory(mp3rootDirectory)) < 0)
- exit(DIRECTORY_UPDATE_EXIT_ERROR);
- }
+ pthread_attr_t attr;
- if (dbUpdated == UPDATE_RETURN_NOUPDATE)
- exit(DIRECTORY_UPDATE_EXIT_NOUPDATE);
-
- /* ignore signals since we don't want them to corrupt the db */
- ignoreSignals();
- if (writeDirectoryDB() < 0) {
- exit(DIRECTORY_UPDATE_EXIT_ERROR);
- }
- exit(DIRECTORY_UPDATE_EXIT_UPDATE);
- } else if (directory_updatePid < 0) {
- unblockSignals();
- ERROR("updateInit: Problems forking()'ing\n");
- commandError(fd, ACK_ERROR_SYSTEM,
- "problems trying to update");
- directory_updatePid = 0;
+ if (progress != UPDATE_PROGRESS_IDLE) {
+ commandError(fd, ACK_ERROR_UPDATE_ALREADY, "already updating");
return -1;
}
- unblockSignals();
+ progress = UPDATE_PROGRESS_RUNNING;
+ pthread_attr_init(&attr);
+ if (pthread_create(&update_thr, &attr, update_task, path_list))
+ FATAL("Failed to spawn update task: %s\n", strerror(errno));
directory_updateJobId++;
if (directory_updateJobId > 1 << 15)
directory_updateJobId = 1;
- DEBUG("updateInit: fork()'d update child for update job id %i\n",
+ DEBUG("updateInit: spawned update thread for update job id %i\n",
(int)directory_updateJobId);
fdprintf(fd, "updating_db: %i\n", (int)directory_updateJobId);
diff --git a/src/directory.h b/src/directory.h
index 40c9d6499..351ebd2cd 100644
--- a/src/directory.h
+++ b/src/directory.h
@@ -34,7 +34,7 @@ typedef struct _Directory {
unsigned stat; /* not needed if ino_t == dev_t == 0 is impossible */
} Directory;
-void readDirectoryDBIfUpdateIsFinished(void);
+void reap_update_task(void);
int isUpdatingDB(void);
diff --git a/src/main.c b/src/main.c
index dcfb7f64d..8028505ab 100644
--- a/src/main.c
+++ b/src/main.c
@@ -439,7 +439,7 @@ int main(int argc, char *argv[])
COMMAND_RETURN_KILL != handlePendingSignals()) {
syncPlayerAndPlaylist();
client_manager_expire();
- readDirectoryDBIfUpdateIsFinished();
+ reap_update_task();
}
write_state_file();
diff --git a/src/sig_handlers.c b/src/sig_handlers.c
index 6b28cb675..e4ac21f22 100644
--- a/src/sig_handlers.c
+++ b/src/sig_handlers.c
@@ -57,7 +57,6 @@ static void chldSigHandler(mpd_unused int sig)
else
break;
}
- directory_sigChldHandler(pid, status);
}
}
--
cgit v1.2.3
From 88424d256390e9c8aad24ed185381dc1297f51d5 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 22 Sep 2008 02:09:16 -0700
Subject: songvec: remove songvec_prune
Any pruned files will be noticed during update and pruned
from the live database, so this inefficient function can
go away and never come back.
---
src/song.c | 2 --
src/songvec.c | 23 -----------------------
src/songvec.h | 2 --
3 files changed, 27 deletions(-)
diff --git a/src/song.c b/src/song.c
index 9be586281..abf8f348b 100644
--- a/src/song.c
+++ b/src/song.c
@@ -234,8 +234,6 @@ void readSongInfoIntoList(FILE * fp, Directory * parentDir)
if (song)
insertSongIntoList(sv, song);
- if (isUpdatingDB()) /* only needed until we get rid of forked update */
- songvec_prune(sv);
}
int updateSongInfo(Song * song)
diff --git a/src/songvec.c b/src/songvec.c
index ac84e7f8e..d416573ce 100644
--- a/src/songvec.c
+++ b/src/songvec.c
@@ -96,26 +96,3 @@ int songvec_write(struct songvec *sv, int fd, int extra)
return 0;
}
-
-/*
- * Removes missing songs from a songvec. This function is only temporary
- * as updating will be moved into a thread and updating shared memory...
- */
-#include "path.h"
-#include "ls.h"
-void songvec_prune(struct songvec *sv)
-{
- int i;
- char tmp[MPD_PATH_MAX];
- struct stat sb;
-
- for (i = sv->nr; --i >= 0; ) {
- Song *song = sv->base[i];
- assert(song);
- if (!myStat(get_song_url(tmp, song), &sb))
- continue;
- songvec_delete(sv, song);
- freeSong(song);
- i = sv->nr;
- }
-}
diff --git a/src/songvec.h b/src/songvec.h
index 5952f871f..abbc9365b 100644
--- a/src/songvec.h
+++ b/src/songvec.h
@@ -21,6 +21,4 @@ void songvec_free(struct songvec *sv);
int songvec_write(struct songvec *sv, int fd, int extra);
-void songvec_prune(struct songvec *sv);
-
#endif /* SONGVEC_H */
--
cgit v1.2.3