From 5a42e7362b3745fb59f0aa49bab624f7fba83eff Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 20 Sep 2008 15:43:35 -0700 Subject: Add prefixcmp() (stol^H^H^H^Hborrowed from git) This allows us to avoid the nasty repetition in strncmp(foo, bar, strlen(foo)). We'll miss out on the compiler optimizing strlen() into sizeof() - 1 for string literals for this; but we don't use this it for performance-critical functions anyways... --- src/utils.c | 10 ++++++++++ src/utils.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/utils.c b/src/utils.c index 332c73587..f23f8c1fa 100644 --- a/src/utils.c +++ b/src/utils.c @@ -268,3 +268,13 @@ void xpthread_cond_destroy(pthread_cond_t *cond) if ((err = pthread_cond_destroy(cond))) FATAL("failed to destroy cond: %s\n", strerror(err)); } + +int prefixcmp(const char *str, const char *prefix) +{ + for (; ; str++, prefix++) + if (!*prefix) + return 0; + else if (*str != *prefix) + return (unsigned char)*prefix - (unsigned char)*str; +} + diff --git a/src/utils.h b/src/utils.h index 0001ba3c8..3958ac9a0 100644 --- a/src/utils.h +++ b/src/utils.h @@ -90,6 +90,8 @@ void xpthread_mutex_destroy(pthread_mutex_t *mutex); void xpthread_cond_destroy(pthread_cond_t *cond); +int prefixcmp(const char *str, const char *prefix); + /* * Work-arounds for braindead APIs that require non-const pointers: * ao_play(), free(), vorbis_comment_add_tag(), iconv() -- cgit v1.2.3 From 95817fee8a01f1cdef2f0fb66d351dd031b91c7e Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 20 Sep 2008 16:07:54 -0700 Subject: start using prefixcmp() LOC reduction and less noise makes things easier for tired old folks to follow. --- src/audio.c | 3 +-- src/directory.c | 23 ++++++++--------------- src/inputPlugins/mp3_plugin.c | 2 +- src/ls.c | 3 +-- src/main.c | 2 +- src/playlist.c | 31 ++++++++----------------------- src/song.c | 11 +++++------ src/volume.c | 12 ++++-------- 8 files changed, 29 insertions(+), 58 deletions(-) diff --git a/src/audio.c b/src/audio.c index 5eacf7422..a10f5cf42 100644 --- a/src/audio.c +++ b/src/audio.c @@ -24,7 +24,6 @@ #include "os_compat.h" #define AUDIO_DEVICE_STATE "audio_device_state:" -#define AUDIO_DEVICE_STATE_LEN (sizeof(AUDIO_DEVICE_STATE)-1) #define AUDIO_BUFFER_SIZE 2*MPD_PATH_MAX static AudioFormat audio_format; @@ -483,7 +482,7 @@ void readAudioDevicesState(FILE *fp) while (myFgets(buffer, AUDIO_BUFFER_SIZE, fp)) { char *c, *name; - if (strncmp(buffer, AUDIO_DEVICE_STATE, AUDIO_DEVICE_STATE_LEN)) + if (prefixcmp(buffer, AUDIO_DEVICE_STATE)) continue; c = strchr(buffer, ':'); diff --git a/src/directory.c b/src/directory.c index 06a39d7bf..dbf578857 100644 --- a/src/directory.c +++ b/src/directory.c @@ -895,22 +895,18 @@ static void readDirectoryInfo(FILE * fp, Directory * directory) ListNode *nodeTemp; while (myFgets(buffer, bufferSize, fp) - && 0 != strncmp(DIRECTORY_END, buffer, strlen(DIRECTORY_END))) { - if (0 == strncmp(DIRECTORY_DIR, buffer, strlen(DIRECTORY_DIR))) { + && prefixcmp(buffer, DIRECTORY_END)) { + if (!prefixcmp(buffer, DIRECTORY_DIR)) { strcpy(key, &(buffer[strlen(DIRECTORY_DIR)])); if (!myFgets(buffer, bufferSize, fp)) FATAL("Error reading db, fgets\n"); /* for compatibility with db's prior to 0.11 */ - if (0 == strncmp(DIRECTORY_MTIME, buffer, - strlen(DIRECTORY_MTIME))) { + if (!prefixcmp(buffer, DIRECTORY_MTIME)) { if (!myFgets(buffer, bufferSize, fp)) FATAL("Error reading db, fgets\n"); } - if (strncmp - (DIRECTORY_BEGIN, buffer, - strlen(DIRECTORY_BEGIN))) { + if (prefixcmp(buffer, DIRECTORY_BEGIN)) FATAL("Error reading db at line: %s\n", buffer); - } name = &(buffer[strlen(DIRECTORY_BEGIN)]); while (nextDirNode && (strcmpRet = @@ -938,7 +934,7 @@ static void readDirectoryInfo(FILE * fp, Directory * directory) } readDirectoryInfo(fp, subDirectory); - } else if (0 == strncmp(SONG_BEGIN, buffer, strlen(SONG_BEGIN))) { + } else if (!prefixcmp(buffer, SONG_BEGIN)) { readSongInfoIntoList(fp, directory->songs, directory); } else { FATAL("Unknown line in db: %s\n", buffer); @@ -1097,16 +1093,13 @@ int readDirectoryDB(void) if (0 == strcmp(DIRECTORY_INFO_BEGIN, buffer)) { while (myFgets(buffer, bufferSize, fp) && 0 != strcmp(DIRECTORY_INFO_END, buffer)) { - if (0 == strncmp(DIRECTORY_MPD_VERSION, buffer, - strlen(DIRECTORY_MPD_VERSION))) + if (!prefixcmp(buffer, DIRECTORY_MPD_VERSION)) { if (foundVersion) FATAL("already found version in db\n"); foundVersion = 1; - } else if (0 == - strncmp(DIRECTORY_FS_CHARSET, buffer, - strlen - (DIRECTORY_FS_CHARSET))) { + } else if (!prefixcmp(buffer, + DIRECTORY_FS_CHARSET)) { char *fsCharset; char *tempCharset; diff --git a/src/inputPlugins/mp3_plugin.c b/src/inputPlugins/mp3_plugin.c index a9dad8904..1d6333fd6 100644 --- a/src/inputPlugins/mp3_plugin.c +++ b/src/inputPlugins/mp3_plugin.c @@ -616,7 +616,7 @@ static int parse_lame(struct lame *lame, struct mad_bitptr *ptr, int *bitlen) /* This is technically incorrect, since the encoder might not be lame. * But there's no other way to determine if this is a lame tag, and we * wouldn't want to go reading a tag that's not there. */ - if (strncmp(lame->encoder, "LAME", 4) != 0) + if (prefixcmp(lame->encoder, "LAME")) return 0; if (sscanf(lame->encoder+4, "%u.%u", diff --git a/src/ls.c b/src/ls.c index f87e09fdb..d7cef0bfe 100644 --- a/src/ls.c +++ b/src/ls.c @@ -89,9 +89,8 @@ int isRemoteUrl(const char *url) while (*urlPrefixes) { count++; - if (strncmp(*urlPrefixes, url, strlen(*urlPrefixes)) == 0) { + if (!prefixcmp(url, *urlPrefixes)) return count; - } urlPrefixes++; } diff --git a/src/main.c b/src/main.c index a5941b848..dcfb7f64d 100644 --- a/src/main.c +++ b/src/main.c @@ -149,7 +149,7 @@ static void parseOptions(int argc, char **argv, Options * options) if (argc > 1) { int i = 1; while (i < argc) { - if (strncmp(argv[i], "--", 2) == 0) { + if (!prefixcmp(argv[i], "--")) { if (strcmp(argv[i], "--help") == 0) { usage(argv); exit(EXIT_SUCCESS); diff --git a/src/playlist.c b/src/playlist.c index e62e71583..d454b4ad2 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -323,8 +323,7 @@ void readPlaylistState(FILE *fp) char buffer[PLAYLIST_BUFFER_SIZE]; while (myFgets(buffer, PLAYLIST_BUFFER_SIZE, fp)) { - if (strncmp(buffer, PLAYLIST_STATE_FILE_STATE, - strlen(PLAYLIST_STATE_FILE_STATE)) == 0) { + if (!prefixcmp(buffer, PLAYLIST_STATE_FILE_STATE)) { if (strcmp(&(buffer[strlen(PLAYLIST_STATE_FILE_STATE)]), PLAYLIST_STATE_FILE_STATE_PLAY) == 0) { state = OB_STATE_PLAY; @@ -335,31 +334,22 @@ void readPlaylistState(FILE *fp) == 0) { state = OB_STATE_PAUSE; } - } else if (strncmp(buffer, PLAYLIST_STATE_FILE_TIME, - strlen(PLAYLIST_STATE_FILE_TIME)) == 0) { + } else if (!prefixcmp(buffer, PLAYLIST_STATE_FILE_TIME)) { seek_time = atoi(&(buffer[strlen(PLAYLIST_STATE_FILE_TIME)])); } else - if (strncmp - (buffer, PLAYLIST_STATE_FILE_REPEAT, - strlen(PLAYLIST_STATE_FILE_REPEAT)) == 0) { + if (!prefixcmp(buffer, PLAYLIST_STATE_FILE_REPEAT)) { if (strcmp (&(buffer[strlen(PLAYLIST_STATE_FILE_REPEAT)]), "1") == 0) { setPlaylistRepeatStatus(1); } else setPlaylistRepeatStatus(0); - } else - if (strncmp - (buffer, PLAYLIST_STATE_FILE_CROSSFADE, - strlen(PLAYLIST_STATE_FILE_CROSSFADE)) == 0) { + } else if (!prefixcmp(buffer, PLAYLIST_STATE_FILE_CROSSFADE)) { ob_set_xfade(atoi(&(buffer [strlen (PLAYLIST_STATE_FILE_CROSSFADE)]))); - } else - if (strncmp - (buffer, PLAYLIST_STATE_FILE_RANDOM, - strlen(PLAYLIST_STATE_FILE_RANDOM)) == 0) { + } else if (!prefixcmp(buffer, PLAYLIST_STATE_FILE_RANDOM)) { if (strcmp (& (buffer @@ -368,20 +358,15 @@ void readPlaylistState(FILE *fp) setPlaylistRandomStatus(1); } else setPlaylistRandomStatus(0); - } else if (strncmp(buffer, PLAYLIST_STATE_FILE_CURRENT, - strlen(PLAYLIST_STATE_FILE_CURRENT)) - == 0) { + } else if (!prefixcmp(buffer, PLAYLIST_STATE_FILE_CURRENT)) { if (strlen(buffer) == strlen(PLAYLIST_STATE_FILE_CURRENT)) state_file_fatal(); current = atoi(&(buffer [strlen (PLAYLIST_STATE_FILE_CURRENT)])); - } else - if (strncmp - (buffer, PLAYLIST_STATE_FILE_PLAYLIST_BEGIN, - strlen(PLAYLIST_STATE_FILE_PLAYLIST_BEGIN) - ) == 0) { + } else if (!prefixcmp(buffer, + PLAYLIST_STATE_FILE_PLAYLIST_BEGIN)) { if (state == OB_STATE_STOP) current = -1; loadPlaylistFromStateFile(fp, buffer, state, diff --git a/src/song.c b/src/song.c index ab900a6b4..ff0ada620 100644 --- a/src/song.c +++ b/src/song.c @@ -218,8 +218,7 @@ static int matchesAnMpdTagItemKey(char *buffer, int *itemType) int i; for (i = 0; i < TAG_NUM_OF_ITEM_TYPES; i++) { - if (0 == strncmp(mpdTagItemKeys[i], buffer, - strlen(mpdTagItemKeys[i]))) { + if (!prefixcmp(buffer, mpdTagItemKeys[i])) { *itemType = i; return 1; } @@ -238,7 +237,7 @@ void readSongInfoIntoList(FILE * fp, SongList * list, Directory * parentDir) int itemType; while (myFgets(buffer, bufferSize, fp) && 0 != strcmp(SONG_END, buffer)) { - if (0 == strncmp(SONG_KEY, buffer, strlen(SONG_KEY))) { + if (!prefixcmp(buffer, SONG_KEY)) { if (song) { insertSongIntoList(list, &nextSongNode, song->url, song); @@ -254,7 +253,7 @@ void readSongInfoIntoList(FILE * fp, SongList * list, Directory * parentDir) /* ignore empty lines (starting with '\0') */ } else if (song == NULL) { FATAL("Problems reading song info\n"); - } else if (0 == strncmp(SONG_FILE, buffer, strlen(SONG_FILE))) { + } else if (!prefixcmp(buffer, SONG_FILE)) { /* we don't need this info anymore song->url = xstrdup(&(buffer[strlen(SONG_FILE)])); */ @@ -268,14 +267,14 @@ void readSongInfoIntoList(FILE * fp, SongList * list, Directory * parentDir) &(buffer [strlen(mpdTagItemKeys[itemType]) + 2])); - } else if (0 == strncmp(SONG_TIME, buffer, strlen(SONG_TIME))) { + } else if (!prefixcmp(buffer, SONG_TIME)) { if (!song->tag) { song->tag = tag_new(); tag_begin_add(song->tag); } song->tag->time = atoi(&(buffer[strlen(SONG_TIME)])); - } else if (0 == strncmp(SONG_MTIME, buffer, strlen(SONG_MTIME))) { + } else if (!prefixcmp(buffer, SONG_MTIME)) { song->mtime = atoi(&(buffer[strlen(SONG_MTIME)])); } else diff --git a/src/volume.c b/src/volume.c index 6d8411165..bdb911916 100644 --- a/src/volume.c +++ b/src/volume.c @@ -511,26 +511,22 @@ int changeVolumeLevel(int change, int rel) void read_sw_volume_state(FILE *fp) { - /* strlen(SW_VOLUME_STATE) + strlen('100') + '\0' */ - #define bufsize 16 - char buf[bufsize]; - const size_t len = strlen(SW_VOLUME_STATE); + char buf[sizeof(SW_VOLUME_STATE) + sizeof("100") - 1]; char *end = NULL; long int sv; if (volume_mixerType != VOLUME_MIXER_TYPE_SOFTWARE) return; - while (myFgets(buf, bufsize, fp)) { - if (strncmp(buf, SW_VOLUME_STATE, len)) + while (myFgets(buf, sizeof(buf), fp)) { + if (prefixcmp(buf, SW_VOLUME_STATE)) continue; - sv = strtol(buf + len, &end, 10); + sv = strtol(buf + strlen(SW_VOLUME_STATE), &end, 10); if (mpd_likely(!*end)) changeSoftwareVolume(sv, 0); else ERROR("Can't parse software volume: %s\n", buf); return; } - #undef bufsize } void save_sw_volume_state(int fd) -- cgit v1.2.3 From 8f475cc0ff4640f60c139e037b38d5c0225e1478 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 20 Sep 2008 16:14:27 -0700 Subject: directory: remove unused updateMp3Directory() function It hasn't been used in many years commit 3a89afdd80f228139554372a83a9d74486acf691 Author: Warren Dukes Date: Sat Nov 20 20:28:32 2004 +0000 remove --update-db option (SVN r2719) --- src/directory.c | 18 ------------------ src/directory.h | 2 -- 2 files changed, 20 deletions(-) diff --git a/src/directory.c b/src/directory.c index dbf578857..404afd1a9 100644 --- a/src/directory.c +++ b/src/directory.c @@ -1146,24 +1146,6 @@ int readDirectoryDB(void) return 0; } -void updateMp3Directory(void) -{ - switch (updateDirectory(mp3rootDirectory)) { - case 0: - /* nothing updated */ - return; - case 1: - if (writeDirectoryDB() < 0) - exit(EXIT_FAILURE); - break; - default: - /* something was updated and db should be written */ - FATAL("problems updating music db\n"); - } - - return; -} - static int traverseAllInSubDirectory(Directory * directory, int (*forEachSong) (Song *, void *), int (*forEachDir) (Directory *, void *), diff --git a/src/directory.h b/src/directory.h index 9553095b3..e881f7a04 100644 --- a/src/directory.h +++ b/src/directory.h @@ -55,8 +55,6 @@ int writeDirectoryDB(void); int readDirectoryDB(void); -void updateMp3Directory(void); - Song *getSongFromDB(const char *file); time_t getDbModTime(void); -- cgit v1.2.3 From 1952b762b0b7024c6a993e62ad957718ac669ac4 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 20 Sep 2008 16:20:48 -0700 Subject: Replace SongList with struct songvec Our linked-list implementation is wasteful and the SongList isn't modified enough to benefit from being a linked list. So use a more compact array of song pointers which saves ~200K on a library with ~9K songs (on x86-32). --- src/Makefile.am | 2 + src/dbUtils.c | 4 +- src/directory.c | 94 +++++++++++++++++++++---------------------- src/directory.h | 3 +- src/song.c | 103 +++++++++++------------------------------------ src/song.h | 9 ++--- src/songvec.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/songvec.h | 26 ++++++++++++ 8 files changed, 226 insertions(+), 136 deletions(-) create mode 100644 src/songvec.c create mode 100644 src/songvec.h diff --git a/src/Makefile.am b/src/Makefile.am index 15efaf78e..88774fff1 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -79,6 +79,7 @@ mpd_headers = \ sig_handlers.h \ sllist.h \ song.h \ + songvec.h \ state_file.h \ stats.h \ tag.h \ @@ -137,6 +138,7 @@ mpd_SOURCES = \ signal_check.c \ sllist.c \ song.c \ + songvec.c \ state_file.c \ stats.c \ tag.c \ diff --git a/src/dbUtils.c b/src/dbUtils.c index 0eb485f1e..1ecb9f608 100644 --- a/src/dbUtils.c +++ b/src/dbUtils.c @@ -50,7 +50,7 @@ static int countSongsInDirectory(Directory * directory, { int *count = (int *)data; - *count += directory->songs->numberOfNodes; + *count += directory->songs.nr; return 0; } @@ -357,7 +357,7 @@ static int sumSavedFilenameMemoryInDirectory(Directory * dir, void *data) return 0; *sum += (strlen(getDirectoryPath(dir)) + 1 - sizeof(Directory *)) * - dir->songs->numberOfNodes; + dir->songs.nr; return 0; } diff --git a/src/directory.c b/src/directory.c index 404afd1a9..c9361fb18 100644 --- a/src/directory.c +++ b/src/directory.c @@ -247,14 +247,14 @@ static Directory *newDirectory(const char *dirname, Directory * parent) { Directory *directory; - directory = xmalloc(sizeof(Directory)); + directory = xcalloc(1, sizeof(Directory)); if (dirname && strlen(dirname)) directory->path = xstrdup(dirname); else directory->path = NULL; directory->subDirectories = newDirectoryList(); - directory->songs = newSongList(); + assert(!directory->songs.base); directory->stat = 0; directory->inode = 0; directory->device = 0; @@ -266,7 +266,7 @@ static Directory *newDirectory(const char *dirname, Directory * parent) static void freeDirectory(Directory * directory) { freeDirectoryList(directory->subDirectories); - freeSongList(directory->songs); + songvec_free(&directory->songs); if (directory->path) free(directory->path); free(directory); @@ -281,13 +281,13 @@ static void freeDirectoryList(DirectoryList * directoryList) static void removeSongFromDirectory(Directory * directory, const char *shortname) { - void *song; + Song *song = songvec_find(&directory->songs, shortname); - if (findInList(directory->songs, shortname, &song)) { + if (song) { char path_max_tmp[MPD_PATH_MAX]; /* wasteful */ - LOG("removing: %s\n", - get_song_url(path_max_tmp, (Song *) song)); - deleteFromList(directory->songs, shortname); + LOG("removing: %s\n", get_song_url(path_max_tmp, song)); + songvec_delete(&directory->songs, song); + freeSong(song); } } @@ -302,7 +302,7 @@ static void deleteEmptyDirectoriesInDirectory(Directory * directory) deleteEmptyDirectoriesInDirectory(subDir); nextNode = node->nextNode; if (subDir->subDirectories->numberOfNodes == 0 && - subDir->songs->numberOfNodes == 0) { + subDir->songs.nr == 0) { deleteNodeFromList(directory->subDirectories, node); } node = nextNode; @@ -317,7 +317,7 @@ static void deleteEmptyDirectoriesInDirectory(Directory * directory) static int updateInDirectory(Directory * directory, const char *shortname, const char *name) { - void *song; + Song *song; void *subDir; struct stat st; @@ -325,14 +325,13 @@ static int updateInDirectory(Directory * directory, return -1; if (S_ISREG(st.st_mode) && hasMusicSuffix(name, 0)) { - if (0 == findInList(directory->songs, shortname, &song)) { + if (!(song = songvec_find(&directory->songs, shortname))) { addToDirectory(directory, shortname, name); return DIRECTORY_RETURN_UPDATE; - } else if (st.st_mtime != ((Song *) song)->mtime) { + } else if (st.st_mtime != song->mtime) { LOG("updating %s\n", name); - if (updateSongInfo((Song *) song) < 0) { + if (updateSongInfo(song) < 0) removeSongFromDirectory(directory, shortname); - } return 1; } } else if (S_ISDIR(st.st_mode)) { @@ -367,6 +366,8 @@ static int removeDeletedFromDirectory(char *path_max_tmp, Directory * directory) ListNode *node, *tmpNode; DirectoryList *subdirs = directory->subDirectories; int ret = 0; + int i; + struct songvec *sv = &directory->songs; node = subdirs->firstNode; while (node) { @@ -387,22 +388,20 @@ static int removeDeletedFromDirectory(char *path_max_tmp, Directory * directory) node = tmpNode; } - node = directory->songs->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); + for (i = sv->nr; --i >= 0; ) { /* cleaner deletes if we go backwards */ + Song *song = sv->base[i]; + if (!song || !song->url) + continue; /* does this happen?, perhaps assert() */ - if (!isFile(path_max_tmp, NULL)) { - removeSongFromDirectory(directory, node->key); - ret = 1; - } + if (dirname) + sprintf(path_max_tmp, "%s/%s", dirname, song->url); + else + strcpy(path_max_tmp, song->url); + + if (!isFile(path_max_tmp, NULL)) { + removeSongFromDirectory(directory, song->url); + ret = 1; } - node = tmpNode; } return ret; @@ -727,12 +726,12 @@ static int addToDirectory(Directory * directory, return -1; } - if (S_ISREG(st.st_mode) && hasMusicSuffix(name, 0)) { - Song *song; - song = addSongToList(directory->songs, shortname, name, - SONG_TYPE_FILE, directory); + if (S_ISREG(st.st_mode) && + hasMusicSuffix(name, 0) && isMusic(name, NULL, 0)) { + Song *song = newSong(shortname, SONG_TYPE_FILE, directory); if (!song) return -1; + songvec_add(&directory->songs, song); LOG("added %s\n", name); return 1; } else if (S_ISDIR(st.st_mode)) { @@ -840,7 +839,7 @@ int printDirectoryInfo(int fd, const char *name) return -1; printDirectoryList(fd, directory->subDirectories); - printSongInfoFromList(fd, directory->songs); + songvec_write(&directory->songs, fd, 0); return 0; } @@ -871,7 +870,7 @@ static void writeDirectoryInfo(int fd, Directory * directory) node = node->nextNode; } - writeSongInfoFromList(fd, directory->songs); + songvec_write(&directory->songs, fd, 1); if (directory->path) { retv = fdprintf(fd, DIRECTORY_END "%s\n", @@ -935,7 +934,7 @@ static void readDirectoryInfo(FILE * fp, Directory * directory) readDirectoryInfo(fp, subDirectory); } else if (!prefixcmp(buffer, SONG_BEGIN)) { - readSongInfoIntoList(fp, directory->songs, directory); + readSongInfoIntoList(fp, directory); } else { FATAL("Unknown line in db: %s\n", buffer); } @@ -954,7 +953,7 @@ static void sortDirectory(Directory * directory) Directory *subDir; sortList(directory->subDirectories); - sortList(directory->songs); + songvec_sort(&directory->songs); while (node != NULL) { subDir = (Directory *) node->data; @@ -1151,8 +1150,7 @@ static int traverseAllInSubDirectory(Directory * directory, int (*forEachDir) (Directory *, void *), void *data) { - ListNode *node = directory->songs->firstNode; - Song *song; + ListNode *node; Directory *dir; int errFlag = 0; @@ -1163,13 +1161,15 @@ static int traverseAllInSubDirectory(Directory * directory, } if (forEachSong) { - while (node != NULL && !errFlag) { - song = (Song *) node->data; - errFlag = forEachSong(song, data); - node = node->nextNode; + int i; + struct songvec *sv = &directory->songs; + Song **sp = sv->base; + + for (i = sv->nr; --i >= 0; ) { + Song *song = *sp++; + if ((errFlag = forEachSong(song, data))) + return errFlag; } - if (errFlag) - return errFlag; } node = directory->subDirectories->firstNode; @@ -1213,7 +1213,7 @@ void initMp3Directory(void) static Song *getSongDetails(const char *file, const char **shortnameRet, Directory ** directoryRet) { - void *song = NULL; + Song *song; Directory *directory; char *dir = NULL; char *duplicated = xstrdup(file); @@ -1240,7 +1240,7 @@ static Song *getSongDetails(const char *file, const char **shortnameRet, return NULL; } - if (!findInList(directory->songs, shortname, &song)) { + if (!(song = songvec_find(&directory->songs, shortname))) { free(duplicated); return NULL; } @@ -1250,7 +1250,7 @@ static Song *getSongDetails(const char *file, const char **shortnameRet, *shortnameRet = shortname; if (directoryRet) *directoryRet = directory; - return (Song *) song; + return song; } Song *getSongFromDB(const char *file) diff --git a/src/directory.h b/src/directory.h index e881f7a04..40c9d6499 100644 --- a/src/directory.h +++ b/src/directory.h @@ -20,13 +20,14 @@ #define DIRECTORY_H #include "song.h" +#include "songvec.h" typedef List DirectoryList; typedef struct _Directory { char *path; DirectoryList *subDirectories; - SongList *songs; + struct songvec songs; struct _Directory *parent; ino_t inode; dev_t device; diff --git a/src/song.c b/src/song.c index ff0ada620..0abf5c9b7 100644 --- a/src/song.c +++ b/src/song.c @@ -27,9 +27,6 @@ #include "inputPlugin.h" #include "myfprintf.h" -#define SONG_KEY "key: " -#define SONG_MTIME "mtime: " - #include "os_compat.h" static Song *newNullSong(void) @@ -152,64 +149,24 @@ int printSongInfo(int fd, Song * song) return 0; } -int printSongInfoFromList(int fd, SongList * list) -{ - ListNode *tempNode = list->firstNode; - - while (tempNode != NULL) { - printSongInfo(fd, (Song *) tempNode->data); - tempNode = tempNode->nextNode; - } - - return 0; -} - -void writeSongInfoFromList(int fd, SongList * list) +static void insertSongIntoList(struct songvec *sv, Song *newsong) { - ListNode *tempNode = list->firstNode; - - fdprintf(fd, SONG_BEGIN "\n"); - - while (tempNode != NULL) { - fdprintf(fd, SONG_KEY "%s\n", tempNode->key); - printSongInfo(fd, (Song *) tempNode->data); - fdprintf(fd, SONG_MTIME "%li\n", - (long)((Song *) tempNode->data)->mtime); - tempNode = tempNode->nextNode; - } - - fdprintf(fd, SONG_END "\n"); -} - -static void insertSongIntoList(SongList * list, ListNode ** nextSongNode, - char *key, Song * song) -{ - ListNode *nodeTemp; - int cmpRet = 0; - - while (*nextSongNode - && (cmpRet = strcmp(key, (*nextSongNode)->key)) > 0) { - nodeTemp = (*nextSongNode)->nextNode; - deleteNodeFromList(list, *nextSongNode); - *nextSongNode = nodeTemp; - } - - if (!(*nextSongNode)) { - insertInList(list, song->url, (void *)song); - } else if (cmpRet == 0) { - Song *tempSong = (Song *) ((*nextSongNode)->data); - if (tempSong->mtime != song->mtime) { - tag_free(tempSong->tag); - tag_end_add(song->tag); - tempSong->tag = song->tag; - tempSong->mtime = song->mtime; - song->tag = NULL; + Song *existing = songvec_find(sv, newsong->url); + + if (!existing) { + songvec_add(sv, newsong); + if (newsong->tag) + tag_end_add(newsong->tag); + } else { /* prevent dupes, just update the existing song info */ + if (existing->mtime != newsong->mtime) { + tag_free(existing->tag); + if (newsong->tag) + tag_end_add(newsong->tag); + existing->tag = newsong->tag; + existing->mtime = newsong->mtime; + newsong->tag = NULL; } - freeJustSong(song); - *nextSongNode = (*nextSongNode)->nextNode; - } else { - insertInListBeforeNode(list, *nextSongNode, -1, song->url, - (void *)song); + freeJustSong(newsong); } } @@ -227,24 +184,18 @@ static int matchesAnMpdTagItemKey(char *buffer, int *itemType) return 0; } -void readSongInfoIntoList(FILE * fp, SongList * list, Directory * parentDir) +void readSongInfoIntoList(FILE * fp, Directory * parentDir) { char buffer[MPD_PATH_MAX + 1024]; int bufferSize = MPD_PATH_MAX + 1024; Song *song = NULL; - ListNode *nextSongNode = list->firstNode; - ListNode *nodeTemp; + struct songvec *sv = &parentDir->songs; int itemType; while (myFgets(buffer, bufferSize, fp) && 0 != strcmp(SONG_END, buffer)) { if (!prefixcmp(buffer, SONG_KEY)) { - if (song) { - insertSongIntoList(list, &nextSongNode, - song->url, song); - if (song->tag != NULL) - tag_end_add(song->tag); - } - + if (song) + insertSongIntoList(sv, song); song = newNullSong(); song->url = xstrdup(buffer + strlen(SONG_KEY)); song->type = SONG_TYPE_FILE; @@ -281,17 +232,9 @@ void readSongInfoIntoList(FILE * fp, SongList * list, Directory * parentDir) FATAL("songinfo: unknown line in db: %s\n", buffer); } - if (song) { - insertSongIntoList(list, &nextSongNode, song->url, song); - if (song->tag != NULL) - tag_end_add(song->tag); - } - - while (nextSongNode) { - nodeTemp = nextSongNode->nextNode; - deleteNodeFromList(list, nextSongNode); - nextSongNode = nodeTemp; - } + if (song) + insertSongIntoList(sv, song); + songvec_prune(sv); } int updateSongInfo(Song * song) diff --git a/src/song.h b/src/song.h index d27596abd..c31c39829 100644 --- a/src/song.h +++ b/src/song.h @@ -24,6 +24,8 @@ #include "tag.h" #include "list.h" +#define SONG_KEY "key: " +#define SONG_MTIME "mtime: " #define SONG_BEGIN "songList begin" #define SONG_END "songList end" @@ -58,12 +60,7 @@ Song *addSongToList(SongList * list, const char *url, const char *utf8path, int printSongInfo(int fd, Song * song); -int printSongInfoFromList(int fd, SongList * list); - -void writeSongInfoFromList(int fd, SongList * list); - -void readSongInfoIntoList(FILE * fp, SongList * list, - struct _Directory *parent); +void readSongInfoIntoList(FILE * fp, struct _Directory *parent); int updateSongInfo(Song * song); diff --git a/src/songvec.c b/src/songvec.c new file mode 100644 index 000000000..ac84e7f8e --- /dev/null +++ b/src/songvec.c @@ -0,0 +1,121 @@ +#include "songvec.h" +#include "myfprintf.h" +#include "utils.h" + +/* Only used for sorting/searchin a songvec, not general purpose compares */ +static int songvec_cmp(const void *s1, const void *s2) +{ + const Song *a = ((const Song * const *)s1)[0]; + const Song *b = ((const Song * const *)s2)[0]; + return strcmp(a->url, b->url); +} + +static size_t sv_size(struct songvec *sv) +{ + return sv->nr * sizeof(Song *); +} + +void songvec_sort(struct songvec *sv) +{ + qsort(sv->base, sv->nr, sizeof(Song *), songvec_cmp); +} + +Song *songvec_find(struct songvec *sv, const char *url) +{ + int i; + + for (i = sv->nr; --i >= 0; ) + if (!strcmp(sv->base[i]->url, url)) + return sv->base[i]; + return NULL; +} + +int songvec_delete(struct songvec *sv, Song *del) +{ + int i; + + for (i = sv->nr; --i >= 0; ) { + if (sv->base[i] != del) + continue; + /* we _don't_ call freeSong() here */ + if (!--sv->nr) { + free(sv->base); + sv->base = NULL; + } else { + memmove(&sv->base[i], &sv->base[i + 1], + (sv->nr - i + 1) * sizeof(Song *)); + sv->base = xrealloc(sv->base, sv_size(sv)); + } + return i; + } + + return -1; /* not found */ +} + +void songvec_add(struct songvec *sv, Song *add) +{ + ++sv->nr; + sv->base = xrealloc(sv->base, sv_size(sv)); + sv->base[sv->nr - 1] = add; +} + +void songvec_free(struct songvec *sv) +{ + free(sv->base); + sv->base = NULL; + sv->nr = 0; +} + +int songvec_write(struct songvec *sv, int fd, int extra) +{ + int i; + Song **sp = sv->base; + + if (extra) { + if (fdprintf(fd, SONG_BEGIN "\n") < 0) + return -1; + + for (i = sv->nr; --i >= 0; ) { + Song *song = *sp++; + if (fdprintf(fd, SONG_KEY "%s\n", song->url) < 0) + return -1; + if (printSongInfo(fd, song) < 0) + return -1; + if (fdprintf(fd, + SONG_MTIME "%li\n", (long)song->mtime) < 0) + return -1; + } + + if (fdprintf(fd, SONG_END "\n") < 0) + return -1; + } else { + for (i = sv->nr; --i >= 0;) + if (printSongInfo(fd, *sp++) < 0) + return -1; + } + + 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 new file mode 100644 index 000000000..5952f871f --- /dev/null +++ b/src/songvec.h @@ -0,0 +1,26 @@ +#ifndef SONGVEC_H +#define SONGVEC_H + +#include "song.h" +#include "os_compat.h" + +struct songvec { + Song **base; + size_t nr; +}; + +void songvec_sort(struct songvec *sv); + +Song *songvec_find(struct songvec *sv, const char *url); + +int songvec_delete(struct songvec *sv, Song *del); + +void songvec_add(struct songvec *sv, Song *add); + +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 From 66a934d068fb6e0aab2043d56a8857d57b6b7596 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 20 Sep 2008 17:22:15 -0700 Subject: workaround race condition on updates with broken signal blocking pthreads with our existing signal blocking/handling is broken, for now just sleep a bit in the child to prevent the CHLD handler from being called too early. Also, improve error reporting when handling SIGCHLD by storing the status to be called in the main task (which can be logged, since we can't do logging inside the sig handler). --- src/directory.c | 89 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/src/directory.c b/src/directory.c index c9361fb18..230dea5dc 100644 --- a/src/directory.c +++ b/src/directory.c @@ -33,6 +33,7 @@ #include "ack.h" #include "myfprintf.h" #include "dbUtils.h" +#include "main_notify.h" #define DIRECTORY_DIR "directory: " #define DIRECTORY_MTIME "mtime: " @@ -55,11 +56,13 @@ static Directory *mp3rootDirectory; static time_t directory_dbModTime; -static volatile int directory_updatePid; +static sig_atomic_t directory_updatePid; -static volatile int directory_reReadDB; +static sig_atomic_t update_exited; -static volatile mpd_uint16 directory_updateJobId; +static sig_atomic_t update_status; + +static sig_atomic_t directory_updateJobId; static DirectoryList *newDirectoryList(void); @@ -109,53 +112,48 @@ static char *getDbFile(void) return param->value; } -static void clearUpdatePid(void) -{ - directory_updatePid = 0; -} - int isUpdatingDB(void) { - if (directory_updatePid > 0 || directory_reReadDB) { - return directory_updateJobId; - } - return 0; + return directory_updatePid > 0 ? directory_updateJobId : 0; } void directory_sigChldHandler(int pid, int status) { if (directory_updatePid == pid) { - if (WIFSIGNALED(status) && WTERMSIG(status) != SIGTERM) { - /* ERROR("update process died from a " - "non-TERM signal: %i\n", WTERMSIG(status)); */ - } else if (!WIFSIGNALED(status)) { - switch (WEXITSTATUS(status)) { - case DIRECTORY_UPDATE_EXIT_UPDATE: - directory_reReadDB = 1; - /* DEBUG("directory_sigChldHandler: " - "updated db\n"); */ - case DIRECTORY_UPDATE_EXIT_NOUPDATE: - /* DEBUG("directory_sigChldHandler: " - "update exited succesffully\n"); */ - break; - /* - default: - ERROR("error updating db\n"); - */ - } - } - clearUpdatePid(); + update_status = status; + update_exited = 1; + wakeup_main_task(); } } void readDirectoryDBIfUpdateIsFinished(void) { - if (directory_reReadDB && 0 == directory_updatePid) { - DEBUG("readDirectoryDB since update finished successfully\n"); - readDirectoryDB(); - playlistVersionChange(); - directory_reReadDB = 0; + int status; + + if (!update_exited) + return; + + 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"); + } } + update_exited = 0; + directory_updatePid = 0; } int updateInit(int fd, List * pathList) @@ -165,8 +163,14 @@ int updateInit(int fd, List * pathList) return -1; } - /* need to block CHLD signal, cause it can exit before we - even get a chance to assign directory_updatePID */ + /* + * 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) { @@ -181,6 +185,13 @@ int updateInit(int fd, List * pathList) 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; -- cgit v1.2.3 From 228736ffb9d88a2910d7ae7df71827d12d7bfeb5 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 20 Sep 2008 17:28:12 -0700 Subject: Don't try to prune unless we're updating Pruning is very expensive and we won't need it in the future anyways. This brings startup back to previous speeds (before songvec changes). --- src/song.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/song.c b/src/song.c index 0abf5c9b7..9be586281 100644 --- a/src/song.c +++ b/src/song.c @@ -234,7 +234,8 @@ void readSongInfoIntoList(FILE * fp, Directory * parentDir) if (song) insertSongIntoList(sv, song); - songvec_prune(sv); + if (isUpdatingDB()) /* only needed until we get rid of forked update */ + songvec_prune(sv); } int updateSongInfo(Song * song) -- cgit v1.2.3