From c5a17794b0bb8112079dd100a334b0175aec4cfa Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 3 Oct 2008 02:19:54 -0700 Subject: song: start avoiding race in updateSongInfo This is still not SMP-safe yet, as it needs at least a barrier before calling tag_free(old_tag). --- src/song.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/song.c b/src/song.c index 5c5024391..dc100899a 100644 --- a/src/song.c +++ b/src/song.c @@ -207,19 +207,22 @@ int updateSongInfo(Song * song) unsigned int next = 0; char path_max_tmp[MPD_PATH_MAX]; char abs_path[MPD_PATH_MAX]; + struct mpd_tag *old_tag = song->tag; + struct mpd_tag *new_tag = NULL; utf8_to_fs_charset(abs_path, get_song_url(path_max_tmp, song)); rmp2amp_r(abs_path, abs_path); - if (song->tag) - tag_free(song->tag); - - song->tag = NULL; - - while (!song->tag && (plugin = isMusic(abs_path, - &(song->mtime), - next++))) { - song->tag = plugin->tagDupFunc(abs_path); + while ((plugin = isMusic(abs_path, &song->mtime, next++))) { + if ((new_tag = plugin->tagDupFunc(abs_path))) + break; + } + if (new_tag && tag_equal(new_tag, old_tag)) { + tag_free(new_tag); + } else { + song->tag = new_tag; + if (old_tag) + tag_free(old_tag); } if (!song->tag || song->tag->time < 0) return -1; -- cgit v1.2.3 From cd1e02c8a88dd3e55cbde3056906dfbd3207f30e Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 3 Oct 2008 18:11:47 -0700 Subject: tag: merge clearMpdTag into tag_free avoid some redundant clearing logic as well, since the tag is getting freed. --- src/tag.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/tag.c b/src/tag.c index 2dcaf4ef8..db446836c 100644 --- a/src/tag.c +++ b/src/tag.c @@ -296,12 +296,12 @@ void tag_clear_items_by_type(struct mpd_tag *tag, enum tag_type type) } } -static void clearMpdTag(struct mpd_tag *tag) +void tag_free(struct mpd_tag *tag) { int i; pthread_mutex_lock(&tag_pool_lock); - for (i = 0; i < tag->numOfItems; i++) + for (i = tag->numOfItems; --i >= 0; ) tag_pool_put_item(tag->items[i]); pthread_mutex_unlock(&tag_pool_lock); @@ -314,16 +314,6 @@ static void clearMpdTag(struct mpd_tag *tag) free(tag->items); } - tag->items = NULL; - - tag->numOfItems = 0; - - tag->time = -1; -} - -void tag_free(struct mpd_tag *tag) -{ - clearMpdTag(tag); free(tag); } -- cgit v1.2.3 From e46bbc95eae9bc75e6d809cdf157637b682765c2 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 4 Oct 2008 02:44:21 -0700 Subject: song: better handling of existing songs when rereading DB Don't reallocate existing tags if they haven't changed. This isn't used as often anymore, but we still take HUP signals to reread the DB if other processes changed it. --- src/song.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/song.c b/src/song.c index dc100899a..4f7b2f8bb 100644 --- a/src/song.c +++ b/src/song.c @@ -123,11 +123,20 @@ static void insertSongIntoList(struct songvec *sv, Song *newsong) 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; + if (tag_equal(existing->tag, newsong->tag)) { + if (newsong->tag) + tag_free(newsong->tag); + } else { + struct mpd_tag *old_tag = existing->tag; + + if (newsong->tag) + tag_end_add(newsong->tag); + existing->tag = newsong->tag; + if (old_tag) + tag_free(old_tag); + } + /* prevent tag_free in freeJustSong */ newsong->tag = NULL; } freeJustSong(newsong); -- cgit v1.2.3 From b84bf082df6ec5a7223c86abb94d799569aed1c1 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 4 Oct 2008 03:41:59 -0700 Subject: directory: reuse existing directory if found on update Instead of allocating a new one, just reuse an existing one if one is found when rereading the DB. This is a small makes the previous commit work on subdirectories of the root music directory. [1] "song: better handling of existing songs when rereading DB" --- src/directory.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/directory.c b/src/directory.c index fc9c6b41d..fb0ca0b07 100644 --- a/src/directory.c +++ b/src/directory.c @@ -723,12 +723,13 @@ static void readDirectoryInfo(FILE * fp, Directory * directory) char buffer[MPD_PATH_MAX * 2]; int bufferSize = MPD_PATH_MAX * 2; char key[MPD_PATH_MAX * 2]; - Directory *subDirectory; char *name; while (myFgets(buffer, bufferSize, fp) && prefixcmp(buffer, DIRECTORY_END)) { if (!prefixcmp(buffer, DIRECTORY_DIR)) { + Directory *subdir; + strcpy(key, &(buffer[strlen(DIRECTORY_DIR)])); if (!myFgets(buffer, bufferSize, fp)) FATAL("Error reading db, fgets\n"); @@ -740,9 +741,13 @@ 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)]); - subDirectory = newDirectory(name, directory); - dirvec_add(&directory->children, subDirectory); - readDirectoryInfo(fp, subDirectory); + if ((subdir = getDirectory(name))) { + assert(subdir->parent == directory); + } else { + subdir = newDirectory(name, directory); + dirvec_add(&directory->children, subdir); + } + readDirectoryInfo(fp, subdir); } else if (!prefixcmp(buffer, SONG_BEGIN)) { readSongInfoIntoList(fp, directory); } else { -- cgit v1.2.3