From b6b3ceb7dd0cd35ed8f45b997a77e0dccfc6839a Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 27 Sep 2008 00:38:03 -0700 Subject: directory.c: kill unnecessary includes --- src/directory.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index f7c426267..0e7933328 100644 --- a/src/directory.c +++ b/src/directory.c @@ -20,16 +20,12 @@ #include "command.h" #include "conf.h" -#include "client.h" -#include "listen.h" #include "log.h" #include "ls.h" #include "path.h" #include "playlist.h" -#include "sig_handlers.h" #include "stats.h" #include "utils.h" -#include "volume.h" #include "ack.h" #include "myfprintf.h" #include "dbUtils.h" -- cgit v1.2.3 From 9c4f0d3a6e36f8901e3a85be6269b9ec652b8eea Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 00:36:03 -0700 Subject: songvec_free => songvec_destroy "free" implies the songvec structure itself is freed, which is not the case. --- src/directory.c | 2 +- src/songvec.c | 2 +- src/songvec.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index 0e7933328..8dff8156e 100644 --- a/src/directory.c +++ b/src/directory.c @@ -221,7 +221,7 @@ static Directory *newDirectory(const char *dirname, Directory * parent) static void freeDirectory(Directory * directory) { freeDirectoryList(directory->subDirectories); - songvec_free(&directory->songs); + songvec_destroy(&directory->songs); if (directory->path) free(directory->path); free(directory); diff --git a/src/songvec.c b/src/songvec.c index 842e7afcd..7ce1285f6 100644 --- a/src/songvec.c +++ b/src/songvec.c @@ -59,7 +59,7 @@ void songvec_add(struct songvec *sv, Song *add) sv->base[sv->nr - 1] = add; } -void songvec_free(struct songvec *sv) +void songvec_destroy(struct songvec *sv) { if (sv->base) { free(sv->base); diff --git a/src/songvec.h b/src/songvec.h index abbc9365b..b823724e7 100644 --- a/src/songvec.h +++ b/src/songvec.h @@ -17,7 +17,7 @@ int songvec_delete(struct songvec *sv, Song *del); void songvec_add(struct songvec *sv, Song *add); -void songvec_free(struct songvec *sv); +void songvec_destroy(struct songvec *sv); int songvec_write(struct songvec *sv, int fd, int extra); -- cgit v1.2.3 From d78458da87bf2563cf5b73db451bb0c9ad17ad84 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 00:39:01 -0700 Subject: directory: remove unused CPP defines We no longer for for updates --- src/directory.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index 8dff8156e..5334aeec8 100644 --- a/src/directory.c +++ b/src/directory.c @@ -40,10 +40,6 @@ #define DIRECTORY_MPD_VERSION "mpd_version: " #define DIRECTORY_FS_CHARSET "fs_charset: " -#define DIRECTORY_UPDATE_EXIT_NOUPDATE 0 -#define DIRECTORY_UPDATE_EXIT_UPDATE 1 -#define DIRECTORY_UPDATE_EXIT_ERROR 2 - enum update_return { UPDATE_RETURN_ERROR = -1, UPDATE_RETURN_NOUPDATE = 0, -- cgit v1.2.3 From 52a56f14cb581febf36b92506f4d0db0ba7cf42c Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 03:38:13 -0700 Subject: directory: replace DirectoryList with dirvec Small memory reduction compared to songvec since most users have much fewer dirs than songs, but still nice to have. --- src/Makefile.am | 1 + src/directory.c | 291 +++++++++++++++++++------------------------------------- src/directory.h | 7 +- src/dirvec.h | 73 ++++++++++++++ 4 files changed, 175 insertions(+), 197 deletions(-) create mode 100644 src/dirvec.h (limited to 'src') diff --git a/src/Makefile.am b/src/Makefile.am index 88774fff1..5962ed583 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -41,6 +41,7 @@ mpd_headers = \ dbUtils.h \ decode.h \ directory.h \ + dirvec.h \ gcc.h \ inputPlugin.h \ inputPlugins/_flac_common.h \ diff --git a/src/directory.c b/src/directory.c index 5334aeec8..3d8f82dc5 100644 --- a/src/directory.c +++ b/src/directory.c @@ -30,6 +30,7 @@ #include "myfprintf.h" #include "dbUtils.h" #include "main_notify.h" +#include "dirvec.h" #define DIRECTORY_DIR "directory: " #define DIRECTORY_MTIME "mtime: " @@ -60,13 +61,9 @@ static pthread_t update_thr; static int directory_updateJobId; -static DirectoryList *newDirectoryList(void); - static int addToDirectory(Directory * directory, const char *shortname, const char *name); -static void freeDirectoryList(DirectoryList * list); - static void freeDirectory(Directory * directory); static enum update_return exploreDirectory(Directory * directory); @@ -189,11 +186,6 @@ static void directory_set_stat(Directory * dir, const struct stat *st) dir->stat = 1; } -static DirectoryList *newDirectoryList(void) -{ - return makeList((ListFreeDataFunc *) freeDirectory, 1); -} - static Directory *newDirectory(const char *dirname, Directory * parent) { Directory *directory; @@ -202,13 +194,6 @@ static Directory *newDirectory(const char *dirname, Directory * parent) if (dirname && strlen(dirname)) directory->path = xstrdup(dirname); - else - directory->path = NULL; - directory->subDirectories = newDirectoryList(); - assert(!directory->songs.base); - directory->stat = 0; - directory->inode = 0; - directory->device = 0; directory->parent = parent; return directory; @@ -216,7 +201,7 @@ static Directory *newDirectory(const char *dirname, Directory * parent) static void freeDirectory(Directory * directory) { - freeDirectoryList(directory->subDirectories); + dirvec_destroy(&directory->children); songvec_destroy(&directory->songs); if (directory->path) free(directory->path); @@ -225,11 +210,6 @@ static void freeDirectory(Directory * directory) /*getDirectoryPath(NULL); */ } -static void freeDirectoryList(DirectoryList * directoryList) -{ - freeList(directoryList); -} - static void removeSongFromDirectory(Directory * directory, const char *shortname) { Song *song = songvec_find(&directory->songs, shortname); @@ -244,27 +224,22 @@ static void removeSongFromDirectory(Directory * directory, const char *shortname static void deleteEmptyDirectoriesInDirectory(Directory * directory) { - ListNode *node = directory->subDirectories->firstNode; - ListNode *nextNode; - Directory *subDir; - - while (node) { - subDir = (Directory *) node->data; - deleteEmptyDirectoriesInDirectory(subDir); - nextNode = node->nextNode; - if (subDir->subDirectories->numberOfNodes == 0 && - subDir->songs.nr == 0) { - deleteNodeFromList(directory->subDirectories, node); - } - node = nextNode; + int i; + struct dirvec *dv = &directory->children; + + for (i = dv->nr; --i >= 0; ) { + deleteEmptyDirectoriesInDirectory(dv->base[i]); + if (!dv->base[i]->children.nr && !dv->base[i]->songs.nr) + dirvec_delete(dv, dv->base[i]); } + if (!dv->nr) + dirvec_destroy(dv); } static enum update_return updateInDirectory(Directory * directory, const char *shortname, const char *name) { Song *song; - void *subDir; struct stat st; if (myStat(name, &st)) @@ -281,10 +256,10 @@ static enum update_return updateInDirectory(Directory * directory, return UPDATE_RETURN_UPDATED; } } else if (S_ISDIR(st.st_mode)) { - if (findInList - (directory->subDirectories, shortname, (void **)&subDir)) { - directory_set_stat((Directory *)subDir, &st); - return updateDirectory((Directory *) subDir); + Directory *subdir = dirvec_find(&directory->children, name); + if (subdir) { + directory_set_stat(subdir, &st); + return updateDirectory(subdir); } else { return addSubDirectoryToDirectory(directory, shortname, name, &st); @@ -305,29 +280,17 @@ removeDeletedFromDirectory(char *path_max_tmp, Directory * directory) { const char *dirname = (directory && directory->path) ? directory->path : NULL; - ListNode *node, *tmpNode; - DirectoryList *subdirs = directory->subDirectories; enum update_return ret = UPDATE_RETURN_NOUPDATE; int i; struct songvec *sv = &directory->songs; + struct dirvec *dv = &directory->children; - node = subdirs->firstNode; - while (node) { - tmpNode = node->nextNode; - if (node->key) { - if (dirname) - sprintf(path_max_tmp, "%s/%s", dirname, - node->key); - else - strcpy(path_max_tmp, node->key); - - if (!isDir(path_max_tmp)) { - LOG("removing directory: %s\n", path_max_tmp); - deleteFromList(subdirs, node->key); - ret = UPDATE_RETURN_UPDATED; - } - } - node = tmpNode; + 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]); + ret = UPDATE_RETURN_UPDATED; } for (i = sv->nr; --i >= 0; ) { /* cleaner deletes if we go backwards */ @@ -355,7 +318,7 @@ static Directory *addDirectoryPathToDB(const char *utf8path, char path_max_tmp[MPD_PATH_MAX]; char *parent; Directory *parentDirectory; - void *directory; + Directory *directory; parent = parent_path(path_max_tmp, utf8path); @@ -371,16 +334,14 @@ static Directory *addDirectoryPathToDB(const char *utf8path, while (*(*shortname) && *(*shortname) == '/') (*shortname)++; - if (!findInList - (parentDirectory->subDirectories, *shortname, &directory)) { + if (!(directory = dirvec_find(&parentDirectory->children, utf8path))) { struct stat st; if (myStat(utf8path, &st) < 0 || inodeFoundInParent(parentDirectory, st.st_ino, st.st_dev)) return NULL; else { directory = newDirectory(utf8path, parentDirectory); - insertInList(parentDirectory->subDirectories, - *shortname, directory); + dirvec_add(&parentDirectory->children, directory); } } @@ -388,7 +349,7 @@ static Directory *addDirectoryPathToDB(const char *utf8path, with potentially the same name */ removeSongFromDirectory(parentDirectory, *shortname); - return (Directory *) directory; + return directory; } static Directory *addParentPathToDB(const char *utf8path, const char **shortname) @@ -446,8 +407,7 @@ static enum update_return updatePath(const char *utf8path) /* if updateDirectory fails, means we should delete it */ else { LOG("removing directory: %s\n", path); - deleteFromList(parentDirectory->subDirectories, - shortname); + dirvec_delete(&parentDirectory->children, directory); ret = UPDATE_RETURN_UPDATED; /* don't return, path maybe a song now */ } @@ -638,7 +598,7 @@ static enum update_return addSubDirectoryToDirectory(Directory * directory, return UPDATE_RETURN_NOUPDATE; } - insertInList(directory->subDirectories, shortname, subDirectory); + dirvec_add(&directory->children, subDirectory); return UPDATE_RETURN_UPDATED; } @@ -676,27 +636,6 @@ void closeMp3Directory(void) freeDirectory(mp3rootDirectory); } -static Directory *findSubDirectory(Directory * directory, const char *name) -{ - void *subDirectory; - char *duplicated = xstrdup(name); - char *key; - - key = strtok(duplicated, "/"); - if (!key) { - free(duplicated); - return NULL; - } - - if (findInList(directory->subDirectories, key, &subDirectory)) { - free(duplicated); - return (Directory *) subDirectory; - } - - free(duplicated); - return NULL; -} - int isRootDirectory(const char *name) { if (name == NULL || name[0] == '\0' || strcmp(name, "/") == 0) { @@ -708,25 +647,34 @@ int isRootDirectory(const char *name) static Directory *getSubDirectory(Directory * directory, const char *name, const char **shortname) { - Directory *subDirectory; - int len; + Directory *cur = directory; + Directory *found = NULL; + char *duplicated; + char *locate; - if (isRootDirectory(name)) { + if (isRootDirectory(name)) return directory; - } - if ((subDirectory = findSubDirectory(directory, name)) == NULL) - return NULL; + duplicated = xstrdup(name); + locate = strchr(duplicated, '/'); + while (1) { + if (locate) + *locate = '\0'; + if (!(found = dirvec_find(&cur->children, duplicated))) + break; + cur = found; + if (!locate) + break; + *locate = '/'; + locate = strchr(locate + 1, '/'); + } - *shortname = name; + free(duplicated); - len = 0; - while (name[len] != '/' && name[len] != '\0') - len++; - while (name[len] == '/') - len++; + if (found && (!(*shortname = strrchr(found->path, '/')))) + *shortname = found->path; - return getSubDirectory(subDirectory, &(name[len]), shortname); + return found; } static Directory *getDirectoryDetails(const char *name, const char **shortname) @@ -743,16 +691,14 @@ static Directory *getDirectory(const char *name) return getSubDirectory(mp3rootDirectory, name, &shortname); } -static int printDirectoryList(int fd, DirectoryList * directoryList) +static int printDirectoryList(int fd, struct dirvec *dv) { - ListNode *node = directoryList->firstNode; - Directory *directory; + size_t i; - while (node != NULL) { - directory = (Directory *) node->data; - fdprintf(fd, "%s%s\n", DIRECTORY_DIR, - getDirectoryPath(directory)); - node = node->nextNode; + for (i = 0; i < dv->nr; ++i) { + if (fdprintf(fd, DIRECTORY_DIR "%s\n", + getDirectoryPath(dv->base[i])) < 0) + return -1; } return 0; @@ -765,16 +711,17 @@ int printDirectoryInfo(int fd, const char *name) if ((directory = getDirectory(name)) == NULL) return -1; - printDirectoryList(fd, directory->subDirectories); + printDirectoryList(fd, &directory->children); songvec_write(&directory->songs, fd, 0); return 0; } +/* TODO error checking */ static void writeDirectoryInfo(int fd, Directory * directory) { - ListNode *node = (directory->subDirectories)->firstNode; - Directory *subDirectory; + struct dirvec *children = &directory->children; + size_t i; int retv; if (directory->path) { @@ -786,17 +733,21 @@ static void writeDirectoryInfo(int fd, Directory * directory) } } - while (node != NULL) { - subDirectory = (Directory *) node->data; - retv = fdprintf(fd, DIRECTORY_DIR "%s\n", node->key); + for (i = 0; i < children->nr; ++i) { + Directory *cur = children->base[i]; + const char *base = strrchr(cur->path, '/'); + + base = base ? base + 1 : cur->path; + assert(*base); + + retv = fdprintf(fd, DIRECTORY_DIR "%s\n", base); if (retv < 0) { - ERROR("Failed to write data to database file: %s\n",strerror(errno)); + ERROR("Failed to write data to database file: %s\n", + strerror(errno)); return; } - writeDirectoryInfo(fd, subDirectory); - node = node->nextNode; + writeDirectoryInfo(fd, cur); } - songvec_write(&directory->songs, fd, 1); if (directory->path) { @@ -815,10 +766,7 @@ static void readDirectoryInfo(FILE * fp, Directory * directory) int bufferSize = MPD_PATH_MAX * 2; char key[MPD_PATH_MAX * 2]; Directory *subDirectory; - int strcmpRet; char *name; - ListNode *nextDirNode = directory->subDirectories->firstNode; - ListNode *nodeTemp; while (myFgets(buffer, bufferSize, fp) && prefixcmp(buffer, DIRECTORY_END)) { @@ -834,31 +782,8 @@ static void readDirectoryInfo(FILE * fp, Directory * directory) if (prefixcmp(buffer, DIRECTORY_BEGIN)) FATAL("Error reading db at line: %s\n", buffer); name = &(buffer[strlen(DIRECTORY_BEGIN)]); - - while (nextDirNode && (strcmpRet = - strcmp(key, - nextDirNode->key)) > 0) { - nodeTemp = nextDirNode->nextNode; - deleteNodeFromList(directory->subDirectories, - nextDirNode); - nextDirNode = nodeTemp; - } - - if (NULL == nextDirNode) { - subDirectory = newDirectory(name, directory); - insertInList(directory->subDirectories, - key, (void *)subDirectory); - } else if (strcmpRet == 0) { - subDirectory = (Directory *) nextDirNode->data; - nextDirNode = nextDirNode->nextNode; - } else { - subDirectory = newDirectory(name, directory); - insertInListBeforeNode(directory-> - subDirectories, - nextDirNode, -1, key, - (void *)subDirectory); - } - + subDirectory = newDirectory(name, directory); + dirvec_add(&directory->children, subDirectory); readDirectoryInfo(fp, subDirectory); } else if (!prefixcmp(buffer, SONG_BEGIN)) { readSongInfoIntoList(fp, directory); @@ -866,27 +791,18 @@ static void readDirectoryInfo(FILE * fp, Directory * directory) FATAL("Unknown line in db: %s\n", buffer); } } - - while (nextDirNode) { - nodeTemp = nextDirNode->nextNode; - deleteNodeFromList(directory->subDirectories, nextDirNode); - nextDirNode = nodeTemp; - } } static void sortDirectory(Directory * directory) { - ListNode *node = directory->subDirectories->firstNode; - Directory *subDir; + int i; + struct dirvec *dv = &directory->children; - sortList(directory->subDirectories); + dirvec_sort(dv); songvec_sort(&directory->songs); - while (node != NULL) { - subDir = (Directory *) node->data; - sortDirectory(subDir); - node = node->nextNode; - } + for (i = dv->nr; --i >= 0; ) + sortDirectory(dv->base[i]); } int checkDirectoryDB(void) @@ -1076,9 +992,9 @@ static int traverseAllInSubDirectory(Directory * directory, int (*forEachDir) (Directory *, void *), void *data) { - ListNode *node; - Directory *dir; + struct dirvec *dv = &directory->children; int errFlag = 0; + size_t j; if (forEachDir) { errFlag = forEachDir(directory, data); @@ -1098,14 +1014,9 @@ static int traverseAllInSubDirectory(Directory * directory, } } - node = directory->subDirectories->firstNode; - - while (node != NULL && !errFlag) { - dir = (Directory *) node->data; - errFlag = traverseAllInSubDirectory(dir, forEachSong, + for (j = 0; !errFlag && j < dv->nr; ++j) + errFlag = traverseAllInSubDirectory(dv->base[j], forEachSong, forEachDir, data); - node = node->nextNode; - } return errFlag; } @@ -1139,43 +1050,33 @@ void initMp3Directory(void) static Song *getSongDetails(const char *file, const char **shortnameRet, Directory ** directoryRet) { - Song *song; + Song *song = NULL; Directory *directory; char *dir = NULL; char *duplicated = xstrdup(file); - char *shortname = duplicated; - char *c = strtok(duplicated, "/"); + char *shortname = strrchr(duplicated, '/'); DEBUG("get song: %s\n", file); - while (c) { - shortname = c; - c = strtok(NULL, "/"); - } - - if (shortname != duplicated) { - for (c = duplicated; c < shortname - 1; c++) { - if (*c == '\0') - *c = '/'; - } + if (!shortname) { + shortname = duplicated; + } else { + *shortname = '\0'; + ++shortname; dir = duplicated; } - if (!(directory = getDirectory(dir))) { - free(duplicated); - return NULL; - } - - if (!(song = songvec_find(&directory->songs, shortname))) { - free(duplicated); - return NULL; - } + if (!(directory = getDirectory(dir))) + goto out; + if (!(song = songvec_find(&directory->songs, shortname))) + goto out; - free(duplicated); if (shortnameRet) - *shortnameRet = shortname; + *shortnameRet = song->url; if (directoryRet) *directoryRet = directory; +out: + free(duplicated); return song; } diff --git a/src/directory.h b/src/directory.h index e2ff3832a..b93e6bddb 100644 --- a/src/directory.h +++ b/src/directory.h @@ -23,11 +23,14 @@ #include "songvec.h" #include "list.h" -typedef List DirectoryList; +struct dirvec { + struct _Directory **base; + size_t nr; +}; typedef struct _Directory { char *path; - DirectoryList *subDirectories; + struct dirvec children; struct songvec songs; struct _Directory *parent; ino_t inode; diff --git a/src/dirvec.h b/src/dirvec.h new file mode 100644 index 000000000..8b2f634e2 --- /dev/null +++ b/src/dirvec.h @@ -0,0 +1,73 @@ +#ifndef DIRVEC_H +#define DIRVEC_H + +#include "directory.h" +#include "os_compat.h" +#include "utils.h" + +static size_t dv_size(struct dirvec *dv) +{ + return dv->nr * sizeof(Directory *); +} + +/* Only used for sorting/searching a dirvec, not general purpose compares */ +static int dirvec_cmp(const void *d1, const void *d2) +{ + const Directory *a = ((const Directory * const *)d1)[0]; + const Directory *b = ((const Directory * const *)d2)[0]; + return strcmp(a->path, b->path); +} + +static void dirvec_sort(struct dirvec *dv) +{ + qsort(dv->base, dv->nr, sizeof(Directory *), dirvec_cmp); +} + +static Directory *dirvec_find(struct dirvec *dv, const char *path) +{ + int i; + + for (i = dv->nr; --i >= 0; ) + if (!strcmp(dv->base[i]->path, path)) + return dv->base[i]; + return NULL; +} + +static int dirvec_delete(struct dirvec *dv, Directory *del) +{ + int i; + + for (i = dv->nr; --i >= 0; ) { + if (dv->base[i] != del) + continue; + /* we _don't_ call freeDirectory() here */ + if (!--dv->nr) { + free(dv->base); + dv->base = NULL; + } else { + memmove(&dv->base[i], &dv->base[i + 1], + (dv->nr - i + 1) * sizeof(Directory *)); + dv->base = xrealloc(dv->base, dv_size(dv)); + } + return i; + } + + return -1; /* not found */ +} + +static void dirvec_add(struct dirvec *dv, Directory *add) +{ + ++dv->nr; + dv->base = xrealloc(dv->base, dv_size(dv)); + dv->base[dv->nr - 1] = add; +} + +static void dirvec_destroy(struct dirvec *dv) +{ + if (dv->base) { + free(dv->base); + dv->base = NULL; + } + dv->nr = 0; +} +#endif /* DIRVEC_H */ -- cgit v1.2.3 From 3ab37f7580f97987f8c4c9787c07715b8279db57 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 03:46:05 -0700 Subject: directory.h: remove directory_sigChldHandler decl We no longer fork for directory updates, so we no longer have children to reap. --- src/directory.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'src') diff --git a/src/directory.h b/src/directory.h index b93e6bddb..fedeb9fa2 100644 --- a/src/directory.h +++ b/src/directory.h @@ -42,8 +42,6 @@ void reap_update_task(void); int isUpdatingDB(void); -void directory_sigChldHandler(int pid, int status); - int updateInit(int fd, List * pathList); void initMp3Directory(void); -- cgit v1.2.3 From 482fe8bf05d9d95fa262be3bd815d686740bf449 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 16:30:33 -0700 Subject: path: add mpd_basename() function This is like basename(3) but with predictable semantics independent of C library or build options used. This is also much more strict and does not account for trailing slashes (mpd should never deal with trailing slashes on internal functions). --- src/path.c | 12 ++++++++++++ src/path.h | 8 ++++++++ 2 files changed, 20 insertions(+) (limited to 'src') diff --git a/src/path.c b/src/path.c index ceb00c5de..8d1b0018d 100644 --- a/src/path.c +++ b/src/path.c @@ -290,3 +290,15 @@ void utf8_to_fs_playlist_path(char *path_max_tmp, const char *utf8path) rpp2app_r(path_max_tmp, path_max_tmp); strncat(path_max_tmp, "." PLAYLIST_FILE_SUFFIX, MPD_PATH_MAX - 1); } + +/* Only takes sanitized paths w/o trailing slashes */ +const char *mpd_basename(const char *path) +{ + const char *ret = strrchr(path, '/'); + + if (!ret) + return path; + ++ret; + assert(*ret != '\0'); + return ret; +} diff --git a/src/path.h b/src/path.h index a91728126..2c5801528 100644 --- a/src/path.h +++ b/src/path.h @@ -88,4 +88,12 @@ void pathcpy_trunc(char *dest, const char *src); */ void utf8_to_fs_playlist_path(char *path_max_tmp, const char *utf8path); +/* + * Like basename(3) but with predictable semantics independent + * of C library or build options used. This is also much more strict + * and does not account for trailing slashes (mpd should never deal with + * trailing slashes on internal functions). + */ +const char *mpd_basename(const char *path); + #endif -- cgit v1.2.3 From 9112a16f0404908f381aa341a00e1fae26e61831 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 18:47:42 -0700 Subject: directory: remove shortname arguments everywhere It was a huge confusing mess of parameter passing around and around. Add a few extra assertions to ensure we're handling parent/child relationships properly. --- src/directory.c | 119 +++++++++++++++++++------------------------------------- 1 file changed, 40 insertions(+), 79 deletions(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index 3d8f82dc5..6effc746d 100644 --- a/src/directory.c +++ b/src/directory.c @@ -61,8 +61,7 @@ static pthread_t update_thr; static int directory_updateJobId; -static int addToDirectory(Directory * directory, - const char *shortname, const char *name); +static int addToDirectory(Directory * directory, const char *name); static void freeDirectory(Directory * directory); @@ -76,17 +75,10 @@ static void removeSongFromDirectory(Directory * directory, const char *shortname); static enum update_return addSubDirectoryToDirectory(Directory * directory, - const char *shortname, const char *name, struct stat *st); -static Directory *getDirectoryDetails(const char *name, - const char **shortname); - static Directory *getDirectory(const char *name); -static Song *getSongDetails(const char *file, const char **shortnameRet, - Directory ** directoryRet); - static enum update_return updatePath(const char *utf8path); static void sortDirectory(Directory * directory); @@ -236,8 +228,8 @@ static void deleteEmptyDirectoriesInDirectory(Directory * directory) dirvec_destroy(dv); } -static enum update_return updateInDirectory(Directory * directory, - const char *shortname, const char *name) +static enum update_return +updateInDirectory(Directory * directory, const char *name) { Song *song; struct stat st; @@ -246,8 +238,10 @@ static enum update_return updateInDirectory(Directory * directory, return UPDATE_RETURN_ERROR; if (S_ISREG(st.st_mode) && hasMusicSuffix(name, 0)) { + const char *shortname = mpd_basename(name); + if (!(song = songvec_find(&directory->songs, shortname))) { - addToDirectory(directory, shortname, name); + addToDirectory(directory, name); return UPDATE_RETURN_UPDATED; } else if (st.st_mtime != song->mtime) { LOG("updating %s\n", name); @@ -258,11 +252,11 @@ static enum update_return updateInDirectory(Directory * directory, } else if (S_ISDIR(st.st_mode)) { Directory *subdir = dirvec_find(&directory->children, name); if (subdir) { + assert(directory == subdir->parent); directory_set_stat(subdir, &st); return updateDirectory(subdir); } else { - return addSubDirectoryToDirectory(directory, shortname, - name, &st); + return addSubDirectoryToDirectory(directory, name, &st); } } @@ -312,8 +306,7 @@ removeDeletedFromDirectory(char *path_max_tmp, Directory * directory) return ret; } -static Directory *addDirectoryPathToDB(const char *utf8path, - const char **shortname) +static Directory *addDirectoryPathToDB(const char *utf8path) { char path_max_tmp[MPD_PATH_MAX]; char *parent; @@ -325,16 +318,14 @@ static Directory *addDirectoryPathToDB(const char *utf8path, if (strlen(parent) == 0) parentDirectory = (void *)mp3rootDirectory; else - parentDirectory = addDirectoryPathToDB(parent, shortname); + parentDirectory = addDirectoryPathToDB(parent); if (!parentDirectory) return NULL; - *shortname = utf8path + strlen(parent); - while (*(*shortname) && *(*shortname) == '/') - (*shortname)++; - - if (!(directory = dirvec_find(&parentDirectory->children, utf8path))) { + if ((directory = dirvec_find(&parentDirectory->children, utf8path))) { + assert(parentDirectory == directory->parent); + } else { struct stat st; if (myStat(utf8path, &st) < 0 || inodeFoundInParent(parentDirectory, st.st_ino, st.st_dev)) @@ -347,12 +338,12 @@ static Directory *addDirectoryPathToDB(const char *utf8path, /* if we're adding directory paths, make sure to delete filenames with potentially the same name */ - removeSongFromDirectory(parentDirectory, *shortname); + removeSongFromDirectory(parentDirectory, mpd_basename(directory->path)); return directory; } -static Directory *addParentPathToDB(const char *utf8path, const char **shortname) +static Directory *addParentPathToDB(const char *utf8path) { char *parent; char path_max_tmp[MPD_PATH_MAX]; @@ -363,15 +354,11 @@ static Directory *addParentPathToDB(const char *utf8path, const char **shortname if (strlen(parent) == 0) parentDirectory = (void *)mp3rootDirectory; else - parentDirectory = addDirectoryPathToDB(parent, shortname); + parentDirectory = addDirectoryPathToDB(parent); if (!parentDirectory) return NULL; - *shortname = utf8path + strlen(parent); - while (*(*shortname) && *(*shortname) == '/') - (*shortname)++; - return (Directory *) parentDirectory; } @@ -380,7 +367,6 @@ static enum update_return updatePath(const char *utf8path) Directory *directory; Directory *parentDirectory; Song *song; - const char *shortname; char *path = sanitizePathDup(utf8path); time_t mtime; enum update_return ret = UPDATE_RETURN_NOUPDATE; @@ -390,7 +376,7 @@ static enum update_return updatePath(const char *utf8path) return UPDATE_RETURN_ERROR; /* if path is in the DB try to update it, or else delete it */ - if ((directory = getDirectoryDetails(path, &shortname))) { + if ((directory = getDirectory(path))) { parentDirectory = directory->parent; /* if this update directory is successfull, we are done */ @@ -411,17 +397,17 @@ static enum update_return updatePath(const char *utf8path) ret = UPDATE_RETURN_UPDATED; /* don't return, path maybe a song now */ } - } else if ((song = getSongDetails(path, &shortname, &parentDirectory))) { + } else if ((song = getSongFromDB(path))) { + parentDirectory = song->parentDir; if (!parentDirectory->stat && statDirectory(parentDirectory) < 0) { free(path); return UPDATE_RETURN_NOUPDATE; } - /* if this song update is successfull, we are done */ - else if (0 == inodeFoundInParent(parentDirectory->parent, + /* if this song update is successful, we are done */ + else if (!inodeFoundInParent(parentDirectory->parent, parentDirectory->inode, - parentDirectory->device) - && song && + parentDirectory->device) && isMusic(get_song_url(path_max_tmp, song), &mtime, 0)) { free(path); if (song->mtime == mtime) @@ -430,13 +416,13 @@ static enum update_return updatePath(const char *utf8path) return UPDATE_RETURN_UPDATED; else { removeSongFromDirectory(parentDirectory, - shortname); + song->url); return UPDATE_RETURN_UPDATED; } } /* if updateDirectory fails, means we should delete it */ else { - removeSongFromDirectory(parentDirectory, shortname); + removeSongFromDirectory(parentDirectory, song->url); ret = UPDATE_RETURN_UPDATED; /* don't return, path maybe a directory now */ } @@ -447,13 +433,13 @@ static enum update_return updatePath(const char *utf8path) * name or vice versa, we need to add it to the db */ if (isDir(path) || isMusic(path, NULL, 0)) { - parentDirectory = addParentPathToDB(path, &shortname); + parentDirectory = addParentPathToDB(path); if (!parentDirectory || (!parentDirectory->stat && statDirectory(parentDirectory) < 0)) { } else if (0 == inodeFoundInParent(parentDirectory->parent, parentDirectory->inode, parentDirectory->device) - && addToDirectory(parentDirectory, shortname, path) + && addToDirectory(parentDirectory, path) > 0) { ret = UPDATE_RETURN_UPDATED; } @@ -506,7 +492,7 @@ static enum update_return updateDirectory(Directory * directory) if (directory->path) utf8 = pfx_dir(path_max_tmp, utf8, strlen(utf8), dirname, strlen(dirname)); - if (updateInDirectory(directory, utf8, path_max_tmp) > 0) + if (updateInDirectory(directory, path_max_tmp) > 0) ret = UPDATE_RETURN_UPDATED; } @@ -545,7 +531,7 @@ static enum update_return exploreDirectory(Directory * directory) if (directory->path) utf8 = pfx_dir(path_max_tmp, utf8, strlen(utf8), dirname, strlen(dirname)); - if (addToDirectory(directory, utf8, path_max_tmp) > 0) + if (addToDirectory(directory, path_max_tmp) > 0) ret = UPDATE_RETURN_UPDATED; } @@ -582,7 +568,6 @@ static int inodeFoundInParent(Directory * parent, ino_t inode, dev_t device) } static enum update_return addSubDirectoryToDirectory(Directory * directory, - const char *shortname, const char *name, struct stat *st) { Directory *subDirectory; @@ -603,8 +588,7 @@ static enum update_return addSubDirectoryToDirectory(Directory * directory, return UPDATE_RETURN_UPDATED; } -static int addToDirectory(Directory * directory, - const char *shortname, const char *name) +static int addToDirectory(Directory * directory, const char *name) { struct stat st; @@ -615,15 +599,16 @@ static int addToDirectory(Directory * directory, if (S_ISREG(st.st_mode) && hasMusicSuffix(name, 0) && isMusic(name, NULL, 0)) { - Song *song = newSong(shortname, SONG_TYPE_FILE, directory); - if (!song) + Song *song; + const char *shortname = mpd_basename(name); + + if (!(song = newSong(shortname, SONG_TYPE_FILE, directory))) return -1; songvec_add(&directory->songs, song); LOG("added %s\n", name); return 1; } else if (S_ISDIR(st.st_mode)) { - return addSubDirectoryToDirectory(directory, shortname, name, - &st); + return addSubDirectoryToDirectory(directory, name, &st); } DEBUG("addToDirectory: %s is not a directory or music\n", name); @@ -644,8 +629,7 @@ int isRootDirectory(const char *name) return 0; } -static Directory *getSubDirectory(Directory * directory, const char *name, - const char **shortname) +static Directory *getSubDirectory(Directory * directory, const char *name) { Directory *cur = directory; Directory *found = NULL; @@ -662,6 +646,7 @@ static Directory *getSubDirectory(Directory * directory, const char *name, *locate = '\0'; if (!(found = dirvec_find(&cur->children, duplicated))) break; + assert(cur == found->parent); cur = found; if (!locate) break; @@ -671,24 +656,12 @@ static Directory *getSubDirectory(Directory * directory, const char *name, free(duplicated); - if (found && (!(*shortname = strrchr(found->path, '/')))) - *shortname = found->path; - return found; } -static Directory *getDirectoryDetails(const char *name, const char **shortname) -{ - *shortname = NULL; - - return getSubDirectory(mp3rootDirectory, name, shortname); -} - static Directory *getDirectory(const char *name) { - const char *shortname; - - return getSubDirectory(mp3rootDirectory, name, &shortname); + return getSubDirectory(mp3rootDirectory, name); } static int printDirectoryList(int fd, struct dirvec *dv) @@ -735,10 +708,7 @@ static void writeDirectoryInfo(int fd, Directory * directory) for (i = 0; i < children->nr; ++i) { Directory *cur = children->base[i]; - const char *base = strrchr(cur->path, '/'); - - base = base ? base + 1 : cur->path; - assert(*base); + const char *base = mpd_basename(cur->path); retv = fdprintf(fd, DIRECTORY_DIR "%s\n", base); if (retv < 0) { @@ -1047,8 +1017,7 @@ void initMp3Directory(void) stats.dbPlayTime = sumSongTimesIn(NULL); } -static Song *getSongDetails(const char *file, const char **shortnameRet, - Directory ** directoryRet) +Song *getSongFromDB(const char *file) { Song *song = NULL; Directory *directory; @@ -1070,21 +1039,13 @@ static Song *getSongDetails(const char *file, const char **shortnameRet, goto out; if (!(song = songvec_find(&directory->songs, shortname))) goto out; + assert(song->parentDir == directory); - if (shortnameRet) - *shortnameRet = song->url; - if (directoryRet) - *directoryRet = directory; out: free(duplicated); return song; } -Song *getSongFromDB(const char *file) -{ - return getSongDetails(file, NULL, NULL); -} - time_t getDbModTime(void) { return directory_dbModTime; -- cgit v1.2.3 From 9fb0f4f7cc3e4b2e1fb203a1949799a6018fb57b Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 19:17:00 -0700 Subject: songvec: songvec_delete takes a const Song pointer We don't modify the Song when we delete it --- src/songvec.c | 2 +- src/songvec.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/songvec.c b/src/songvec.c index 7ce1285f6..579ac7033 100644 --- a/src/songvec.c +++ b/src/songvec.c @@ -30,7 +30,7 @@ Song *songvec_find(struct songvec *sv, const char *url) return NULL; } -int songvec_delete(struct songvec *sv, Song *del) +int songvec_delete(struct songvec *sv, const Song *del) { int i; diff --git a/src/songvec.h b/src/songvec.h index b823724e7..fb5c38c8b 100644 --- a/src/songvec.h +++ b/src/songvec.h @@ -13,7 +13,7 @@ void songvec_sort(struct songvec *sv); Song *songvec_find(struct songvec *sv, const char *url); -int songvec_delete(struct songvec *sv, Song *del); +int songvec_delete(struct songvec *sv, const Song *del); void songvec_add(struct songvec *sv, Song *add); -- cgit v1.2.3 From 2470476a2dba60977cb2f982c031af0d4d49d363 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 19:22:38 -0700 Subject: playlist: deleteASongFromPlaylist takes a const Song * We don't change the song pointer there, either. --- src/playlist.c | 2 +- src/playlist.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/playlist.c b/src/playlist.c index 0e35bfbdb..af0aaccc0 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -825,7 +825,7 @@ enum playlist_result deleteFromPlaylistById(int id) return deleteFromPlaylist(playlist.idToPosition[id]); } -void deleteASongFromPlaylist(Song * song) +void deleteASongFromPlaylist(const Song * song) { int i; diff --git a/src/playlist.h b/src/playlist.h index 560a4d1e2..6b5fe0cd7 100644 --- a/src/playlist.h +++ b/src/playlist.h @@ -92,7 +92,7 @@ enum playlist_result savePlaylist(const char *utf8file); enum playlist_result deletePlaylist(const char *utf8file); -void deleteASongFromPlaylist(Song * song); +void deleteASongFromPlaylist(const Song * song); enum playlist_result moveSongInPlaylist(int from, int to); -- cgit v1.2.3 From 60f52cfb620cf3d273122036f5e23783d267dc61 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 19:43:28 -0700 Subject: directory: remove "Mp3" references MPD has supported more audio formats than just MP3 for over five years... --- src/directory.c | 34 +++++++++++++++++----------------- src/directory.h | 4 ++-- src/main.c | 6 +++--- 3 files changed, 22 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index 6effc746d..4434e6e0a 100644 --- a/src/directory.c +++ b/src/directory.c @@ -53,7 +53,7 @@ enum update_progress { UPDATE_PROGRESS_DONE = 2 } progress; -static Directory *mp3rootDirectory; +static Directory *music_root; static time_t directory_dbModTime; @@ -137,7 +137,7 @@ static void * update_task(void *arg) } freeList(path_list); } else { - ret = updateDirectory(mp3rootDirectory); + ret = updateDirectory(music_root); } if (ret == UPDATE_RETURN_UPDATED && writeDirectoryDB() < 0) @@ -316,7 +316,7 @@ static Directory *addDirectoryPathToDB(const char *utf8path) parent = parent_path(path_max_tmp, utf8path); if (strlen(parent) == 0) - parentDirectory = (void *)mp3rootDirectory; + parentDirectory = music_root; else parentDirectory = addDirectoryPathToDB(parent); @@ -352,7 +352,7 @@ static Directory *addParentPathToDB(const char *utf8path) parent = parent_path(path_max_tmp, utf8path); if (strlen(parent) == 0) - parentDirectory = (void *)mp3rootDirectory; + parentDirectory = music_root; else parentDirectory = addDirectoryPathToDB(parent); @@ -386,7 +386,7 @@ static enum update_return updatePath(const char *utf8path) return ret; } /* we don't want to delete the root directory */ - else if (directory == mp3rootDirectory) { + else if (directory == music_root) { free(path); return UPDATE_RETURN_NOUPDATE; } @@ -616,9 +616,9 @@ static int addToDirectory(Directory * directory, const char *name) return -1; } -void closeMp3Directory(void) +void directory_finish(void) { - freeDirectory(mp3rootDirectory); + freeDirectory(music_root); } int isRootDirectory(const char *name) @@ -661,7 +661,7 @@ static Directory *getSubDirectory(Directory * directory, const char *name) static Directory *getDirectory(const char *name) { - return getSubDirectory(mp3rootDirectory, name); + return getSubDirectory(music_root, name); } static int printDirectoryList(int fd, struct dirvec *dv) @@ -842,11 +842,11 @@ int writeDirectoryDB(void) struct stat st; DEBUG("removing empty directories from DB\n"); - deleteEmptyDirectoriesInDirectory(mp3rootDirectory); + deleteEmptyDirectoriesInDirectory(music_root); DEBUG("sorting DB\n"); - sortDirectory(mp3rootDirectory); + sortDirectory(music_root); DEBUG("writing DB\n"); @@ -867,7 +867,7 @@ int writeDirectoryDB(void) DIRECTORY_FS_CHARSET "%s\n" DIRECTORY_INFO_END "\n", getFsCharset()); - writeDirectoryInfo(fd, mp3rootDirectory); + writeDirectoryInfo(fd, music_root); xclose(fd); @@ -883,8 +883,8 @@ int readDirectoryDB(void) char *dbFile = getDbFile(); struct stat st; - if (!mp3rootDirectory) - mp3rootDirectory = newDirectory(NULL, NULL); + if (!music_root) + music_root = newDirectory(NULL, NULL); while (!(fp = fopen(dbFile, "r")) && errno == EINTR) ; if (fp == NULL) { ERROR("unable to open db file \"%s\": %s\n", @@ -945,7 +945,7 @@ int readDirectoryDB(void) DEBUG("reading DB\n"); - readDirectoryInfo(fp, mp3rootDirectory); + readDirectoryInfo(fp, music_root); while (fclose(fp) && errno == EINTR) ; stats.numberOfSongs = countSongsIn(NULL); @@ -1009,10 +1009,10 @@ int traverseAllIn(const char *name, data); } -void initMp3Directory(void) +void directory_init(void) { - mp3rootDirectory = newDirectory(NULL, NULL); - exploreDirectory(mp3rootDirectory); + music_root = newDirectory(NULL, NULL); + exploreDirectory(music_root); stats.numberOfSongs = countSongsIn(NULL); stats.dbPlayTime = sumSongTimesIn(NULL); } diff --git a/src/directory.h b/src/directory.h index fedeb9fa2..5c4c76a25 100644 --- a/src/directory.h +++ b/src/directory.h @@ -44,9 +44,9 @@ int isUpdatingDB(void); int updateInit(int fd, List * pathList); -void initMp3Directory(void); +void directory_init(void); -void closeMp3Directory(void); +void directory_finish(void); int isRootDirectory(const char *name); diff --git a/src/main.c b/src/main.c index 8028505ab..c0acc260b 100644 --- a/src/main.c +++ b/src/main.c @@ -277,7 +277,7 @@ static void openDB(Options * options, char *argv0) flushWarningLog(); if (checkDirectoryDB() < 0) exit(EXIT_FAILURE); - initMp3Directory(); + directory_init(); if (writeDirectoryDB() < 0) exit(EXIT_FAILURE); if (options->createDB) @@ -450,8 +450,8 @@ int main(int argc, char *argv[]) finishPlaylist(); start = clock(); - closeMp3Directory(); - DEBUG("closeMp3Directory took %f seconds\n", + directory_finish(); + DEBUG("directory_finish took %f seconds\n", ((float)(clock()-start))/CLOCKS_PER_SEC); finishNormalization(); -- cgit v1.2.3 From 5fccca8030fd60cb6b9a4bfe99e014cef6e44e38 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 19:46:19 -0700 Subject: directory: make it clear that DIRECTORY_MTIME is deprecated A long time ago in an mpd far away... --- src/directory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index 4434e6e0a..6c185da7b 100644 --- a/src/directory.c +++ b/src/directory.c @@ -33,7 +33,7 @@ #include "dirvec.h" #define DIRECTORY_DIR "directory: " -#define DIRECTORY_MTIME "mtime: " +#define DIRECTORY_MTIME "mtime: " /* DEPRECATED, noop-read-only */ #define DIRECTORY_BEGIN "begin: " #define DIRECTORY_END "end: " #define DIRECTORY_INFO_BEGIN "info_begin" -- cgit v1.2.3 From 1426cde428144dda30a3848e0a8030bf4e052681 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 19:56:17 -0700 Subject: directory: writeDirectoryInfo propagates errors If a write failed, it's a good sign subsequent writes will fail, too, so propgate errors all the way up the stack. --- src/directory.c | 47 +++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index 6c185da7b..56f31bd0f 100644 --- a/src/directory.c +++ b/src/directory.c @@ -691,43 +691,32 @@ int printDirectoryInfo(int fd, const char *name) } /* TODO error checking */ -static void writeDirectoryInfo(int fd, Directory * directory) +static int writeDirectoryInfo(int fd, Directory * directory) { struct dirvec *children = &directory->children; size_t i; - int retv; - - if (directory->path) { - retv = fdprintf(fd, DIRECTORY_BEGIN "%s\n", - getDirectoryPath(directory)); - if (retv < 0) { - ERROR("Failed to write data to database file: %s\n",strerror(errno)); - return; - } - } + + if (directory->path && + fdprintf(fd, DIRECTORY_BEGIN "%s\n", + getDirectoryPath(directory)) < 0) + return -1; for (i = 0; i < children->nr; ++i) { Directory *cur = children->base[i]; const char *base = mpd_basename(cur->path); - retv = fdprintf(fd, DIRECTORY_DIR "%s\n", base); - if (retv < 0) { - ERROR("Failed to write data to database file: %s\n", - strerror(errno)); - return; - } - writeDirectoryInfo(fd, cur); + if (fdprintf(fd, DIRECTORY_DIR "%s\n", base) < 0) + return -1; + if (writeDirectoryInfo(fd, cur) < 0) + return -1; } songvec_write(&directory->songs, fd, 1); - if (directory->path) { - retv = fdprintf(fd, DIRECTORY_END "%s\n", - getDirectoryPath(directory)); - if (retv < 0) { - ERROR("Failed to write data to database file: %s\n",strerror(errno)); - return; - } - } + if (directory->path && + fdprintf(fd, DIRECTORY_END "%s\n", + getDirectoryPath(directory)) < 0) + return -1; + return 0; } static void readDirectoryInfo(FILE * fp, Directory * directory) @@ -867,8 +856,10 @@ int writeDirectoryDB(void) DIRECTORY_FS_CHARSET "%s\n" DIRECTORY_INFO_END "\n", getFsCharset()); - writeDirectoryInfo(fd, music_root); - + if (writeDirectoryInfo(fd, music_root) < 0) + ERROR("Failed to write to database file: %s\n", + strerror(errno)); + return -1; xclose(fd); if (stat(dbFile, &st) == 0) -- cgit v1.2.3 From e7203ca8349f03c5364a97b9dee68a83e4d506dc Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 20:01:32 -0700 Subject: directory: isRootDirectory() is a one-liner Improving the signal to noise ratio... --- src/directory.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index 56f31bd0f..124ea194e 100644 --- a/src/directory.c +++ b/src/directory.c @@ -623,10 +623,7 @@ void directory_finish(void) int isRootDirectory(const char *name) { - if (name == NULL || name[0] == '\0' || strcmp(name, "/") == 0) { - return 1; - } - return 0; + return (!name || name[0] == '\0' || !strcmp(name, "/")); } static Directory *getSubDirectory(Directory * directory, const char *name) -- cgit v1.2.3 From 887f97b868886a66d7bb2fcd0e1fd6810297299d Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 28 Sep 2008 20:29:40 -0700 Subject: clean up updateInit calling and error handling Move error reporting to command.c so directory.c does not deal with client error handling any more. --- src/command.c | 20 ++++++++++++++++---- src/directory.c | 12 ++++-------- src/directory.h | 3 ++- 3 files changed, 22 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index 616132bea..ce41abb64 100644 --- a/src/command.c +++ b/src/command.c @@ -799,6 +799,16 @@ static int handlePlaylistMove(int fd, mpd_unused int *permission, return print_playlist_result(fd, result); } +static int print_update_result(int fd, int ret) +{ + if (ret >= 0) { + fdprintf(fd, "updating_db: %i\n", ret); + return 0; + } + commandError(fd, ACK_ERROR_UPDATE_ALREADY, "already updating"); + return -1; +} + static int listHandleUpdate(int fd, mpd_unused int *permission, mpd_unused int argc, @@ -818,7 +828,7 @@ static int listHandleUpdate(int fd, nextCmd = getCommandEntryFromString(next->data, permission); if (cmd != nextCmd) - return updateInit(fd, pathList); + return print_update_result(fd, updateInit(pathList)); return 0; } @@ -826,12 +836,14 @@ static int listHandleUpdate(int fd, static int handleUpdate(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { + List *pathList = NULL; + if (argc == 2) { - List *pathList = makeList(NULL, 1); + pathList = makeList(NULL, 1); insertInList(pathList, argv[1], NULL); - return updateInit(fd, pathList); } - return updateInit(fd, NULL); + + return print_update_result(fd, updateInit(pathList)); } static int handleNext(mpd_unused int fd, mpd_unused int *permission, diff --git a/src/directory.c b/src/directory.c index 124ea194e..a332316ff 100644 --- a/src/directory.c +++ b/src/directory.c @@ -18,7 +18,6 @@ #include "directory.h" -#include "command.h" #include "conf.h" #include "log.h" #include "ls.h" @@ -148,14 +147,13 @@ out: return (void *)ret; } -int updateInit(int fd, List * path_list) +int updateInit(List * path_list) { pthread_attr_t attr; - if (progress != UPDATE_PROGRESS_IDLE) { - commandError(fd, ACK_ERROR_UPDATE_ALREADY, "already updating"); + if (progress != UPDATE_PROGRESS_IDLE) return -1; - } + progress = UPDATE_PROGRESS_RUNNING; pthread_attr_init(&attr); @@ -166,9 +164,7 @@ int updateInit(int fd, List * path_list) directory_updateJobId = 1; DEBUG("updateInit: spawned update thread for update job id %i\n", (int)directory_updateJobId); - fdprintf(fd, "updating_db: %i\n", (int)directory_updateJobId); - - return 0; + return directory_updateJobId; } static void directory_set_stat(Directory * dir, const struct stat *st) diff --git a/src/directory.h b/src/directory.h index 5c4c76a25..9161efc38 100644 --- a/src/directory.h +++ b/src/directory.h @@ -42,7 +42,8 @@ void reap_update_task(void); int isUpdatingDB(void); -int updateInit(int fd, List * pathList); +/* returns the non-negative update job ID on success, -1 on error */ +int updateInit(List * pathList); void directory_init(void); -- cgit v1.2.3 From 659a543da853bcb28c22df300a93bd22dc9ae877 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 29 Sep 2008 02:48:09 -0700 Subject: update: move path sanitation up the stack to avoid extra copies Remove yet another use of our old malloc-happy linked list implementation and replace it with a simple array of strings. This also implements more eager error handling of invalid paths (still centralized in updateInit) so we can filter out bad paths before we spawn a thread. This also does its part to fix the "update" command inside list mode which lost its static variable in ada24f9a921ff95d874195acf253b5a9dd12213d (although it was broken and requires the fix in 769939b62f7557f8e7c483223d68a8b39af43e37, too). --- src/command.c | 40 +++++++++++++++++++++++++--------------- src/directory.c | 38 ++++++++++++++++++++++++++------------ src/directory.h | 10 +++++++--- 3 files changed, 58 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index ce41abb64..70bfa78b5 100644 --- a/src/command.c +++ b/src/command.c @@ -36,6 +36,7 @@ #include "os_compat.h" #include "player_error.h" #include "outputBuffer.h" +#include "path.h" #define COMMAND_PLAY "play" #define COMMAND_PLAYID "playid" @@ -805,7 +806,10 @@ static int print_update_result(int fd, int ret) fdprintf(fd, "updating_db: %i\n", ret); return 0; } - commandError(fd, ACK_ERROR_UPDATE_ALREADY, "already updating"); + if (ret == -2) + commandError(fd, ACK_ERROR_ARG, "invalid path"); + else + commandError(fd, ACK_ERROR_UPDATE_ALREADY, "already updating"); return -1; } @@ -815,20 +819,28 @@ static int listHandleUpdate(int fd, char *argv[], struct strnode *cmdnode, CommandEntry * cmd) { - List *pathList = makeList(NULL, 1); + static char **pathv; + static int pathc; CommandEntry *nextCmd = NULL; struct strnode *next = cmdnode->next; + int last = pathc++; - if (argc == 2) - insertInList(pathList, argv[1], NULL); - else - insertInList(pathList, "", NULL); + pathv = xrealloc(pathv, pathc * sizeof(char *)); + pathv[last] = sanitizePathDup(argc == 2 ? argv[1] : ""); if (next) nextCmd = getCommandEntryFromString(next->data, permission); - if (cmd != nextCmd) - return print_update_result(fd, updateInit(pathList)); + 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; } @@ -836,14 +848,12 @@ static int listHandleUpdate(int fd, static int handleUpdate(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - List *pathList = NULL; + char *pathv[1]; - if (argc == 2) { - pathList = makeList(NULL, 1); - insertInList(pathList, argv[1], NULL); - } - - return print_update_result(fd, updateInit(pathList)); + assert(argc <= 2); + if (argc == 2) + pathv[0] = sanitizePathDup(argv[1]); + return print_update_result(fd, updateInit(argc - 1, pathv)); } static int handleNext(mpd_unused int fd, mpd_unused int *permission, diff --git a/src/directory.c b/src/directory.c index a332316ff..8d36985dc 100644 --- a/src/directory.c +++ b/src/directory.c @@ -114,16 +114,15 @@ void reap_update_task(void) progress = UPDATE_PROGRESS_IDLE; } -static void * update_task(void *arg) +/* @argv represents a null-terminated array of (null-terminated) strings */ +static void * update_task(void *argv) { - List *path_list = (List *)arg; enum update_return ret = UPDATE_RETURN_NOUPDATE; - if (path_list) { - ListNode *node = path_list->firstNode; - - while (node) { - switch (updatePath(node->key)) { + if (argv) { + char **pathv; + for (pathv = (char **)argv; *pathv; pathv++) { + switch (updatePath(*pathv)) { case UPDATE_RETURN_ERROR: ret = UPDATE_RETURN_ERROR; goto out; @@ -132,9 +131,9 @@ static void * update_task(void *arg) case UPDATE_RETURN_UPDATED: ret = UPDATE_RETURN_UPDATED; } - node = node->nextNode; + free(*pathv); } - freeList(path_list); + free(argv); } else { ret = updateDirectory(music_root); } @@ -147,17 +146,32 @@ out: return (void *)ret; } -int updateInit(List * path_list) +int updateInit(int argc, char *argv[]) { pthread_attr_t attr; + char **pathv = NULL; + int i; - if (progress != UPDATE_PROGRESS_IDLE) + 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; + } 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, path_list)) + if (pthread_create(&update_thr, &attr, update_task, pathv)) FATAL("Failed to spawn update task: %s\n", strerror(errno)); directory_updateJobId++; if (directory_updateJobId > 1 << 15) diff --git a/src/directory.h b/src/directory.h index 9161efc38..eb1b35aa7 100644 --- a/src/directory.h +++ b/src/directory.h @@ -21,7 +21,6 @@ #include "song.h" #include "songvec.h" -#include "list.h" struct dirvec { struct _Directory **base; @@ -42,8 +41,13 @@ void reap_update_task(void); int isUpdatingDB(void); -/* returns the non-negative update job ID on success, -1 on error */ -int updateInit(List * pathList); +/* + * 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 + */ +int updateInit(int argc, char *argv[]); void directory_init(void); -- cgit v1.2.3 From dde461fe6e012a62ee47cf5f3bfc022b650b6bf5 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 29 Sep 2008 02:58:29 -0700 Subject: directory: remove redundant sanitizePathDup We already sanitize and duplicated our paths before calling updateInit() to get pre-pthread_create() error-checking along with heap allocation reduction because we don't have to redupe because our parent stack went away. --- src/directory.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/directory.c b/src/directory.c index 8d36985dc..fc9c6b41d 100644 --- a/src/directory.c +++ b/src/directory.c @@ -377,41 +377,36 @@ static enum update_return updatePath(const char *utf8path) Directory *directory; Directory *parentDirectory; Song *song; - char *path = sanitizePathDup(utf8path); time_t mtime; enum update_return ret = UPDATE_RETURN_NOUPDATE; char path_max_tmp[MPD_PATH_MAX]; - if (NULL == path) - return UPDATE_RETURN_ERROR; + assert(utf8path); /* if path is in the DB try to update it, or else delete it */ - if ((directory = getDirectory(path))) { + if ((directory = getDirectory(utf8path))) { parentDirectory = directory->parent; /* if this update directory is successfull, we are done */ if ((ret = updateDirectory(directory)) >= 0) { - free(path); sortDirectory(directory); return ret; } /* we don't want to delete the root directory */ else if (directory == music_root) { - free(path); return UPDATE_RETURN_NOUPDATE; } /* if updateDirectory fails, means we should delete it */ else { - LOG("removing directory: %s\n", path); + LOG("removing directory: %s\n", utf8path); dirvec_delete(&parentDirectory->children, directory); ret = UPDATE_RETURN_UPDATED; /* don't return, path maybe a song now */ } - } else if ((song = getSongFromDB(path))) { + } else if ((song = getSongFromDB(utf8path))) { parentDirectory = song->parentDir; if (!parentDirectory->stat && statDirectory(parentDirectory) < 0) { - free(path); return UPDATE_RETURN_NOUPDATE; } /* if this song update is successful, we are done */ @@ -419,7 +414,6 @@ static enum update_return updatePath(const char *utf8path) parentDirectory->inode, parentDirectory->device) && isMusic(get_song_url(path_max_tmp, song), &mtime, 0)) { - free(path); if (song->mtime == mtime) return UPDATE_RETURN_NOUPDATE; else if (updateSongInfo(song) == 0) @@ -442,21 +436,19 @@ static enum update_return updatePath(const char *utf8path) * Also, if by chance a directory was replaced by a file of the same * name or vice versa, we need to add it to the db */ - if (isDir(path) || isMusic(path, NULL, 0)) { - parentDirectory = addParentPathToDB(path); + if (isDir(utf8path) || isMusic(utf8path, NULL, 0)) { + parentDirectory = addParentPathToDB(utf8path); if (!parentDirectory || (!parentDirectory->stat && statDirectory(parentDirectory) < 0)) { } else if (0 == inodeFoundInParent(parentDirectory->parent, parentDirectory->inode, parentDirectory->device) - && addToDirectory(parentDirectory, path) + && addToDirectory(parentDirectory, utf8path) > 0) { ret = UPDATE_RETURN_UPDATED; } } - free(path); - return ret; } -- cgit v1.2.3