From 6e8200227f617b7604a01f372460661d8ec5da32 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 4 Sep 2008 01:28:43 -0700 Subject: tag: introduce handy items_size() function Trying to read or remember "tag->numOfItems * sizeof(*tag->items)" requires too much thinking and mental effort on my part. Also, favor "sizeof(struct mpd_tag)" over "sizeof(*tag->items)" because the former is easier to read and follow, even though the latter is easier to modify if the items member changes to a different type. --- src/tag.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/tag.c b/src/tag.c index 0e0ff1ab7..dc2bab2d6 100644 --- a/src/tag.c +++ b/src/tag.c @@ -55,6 +55,11 @@ const char *mpdTagItemKeys[TAG_NUM_OF_ITEM_TYPES] = { static mpd_sint8 ignoreTagItems[TAG_NUM_OF_ITEM_TYPES]; +static size_t items_size(const struct mpd_tag *tag) +{ + return (tag->numOfItems * sizeof(struct tag_item)); +} + void tag_lib_init(void) { int quit = 0; @@ -271,8 +276,7 @@ static void deleteItem(struct mpd_tag *tag, int idx) } if (tag->numOfItems > 0) { - tag->items = xrealloc(tag->items, - tag->numOfItems * sizeof(*tag->items)); + tag->items = xrealloc(tag->items, items_size(tag)); } else { free(tag->items); tag->items = NULL; @@ -334,7 +338,7 @@ struct mpd_tag *tag_dup(const struct mpd_tag *tag) ret = tag_new(); ret->time = tag->time; ret->numOfItems = tag->numOfItems; - ret->items = xmalloc(ret->numOfItems * sizeof(ret->items[0])); + ret->items = ret->numOfItems > 0 ? xmalloc(items_size(tag)) : NULL; for (i = 0; i < tag->numOfItems; i++) { ret->items[i] = tag_pool_dup_item(tag->items[i]); @@ -404,10 +408,8 @@ void tag_end_add(struct mpd_tag *tag) if (tag->numOfItems > 0) { /* copy the tag items from the bulk list over to a new list (which fits exactly) */ - tag->items = xmalloc(tag->numOfItems * - sizeof(tag->items[0])); - memcpy(tag->items, bulk.items, - tag->numOfItems * sizeof(tag->items[0])); + tag->items = xmalloc(items_size(tag)); + memcpy(tag->items, bulk.items, items_size(tag)); } else tag->items = NULL; } @@ -439,15 +441,14 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, if (tag->items != bulk.items) /* bulk mode disabled */ - tag->items = xrealloc(tag->items, - tag->numOfItems * sizeof(*tag->items)); + tag->items = xrealloc(tag->items, items_size(tag)); else if (tag->numOfItems >= BULK_MAX) { /* bulk list already full - switch back to non-bulk */ assert(bulk.busy); - tag->items = xmalloc(tag->numOfItems * sizeof(tag->items[0])); + tag->items = xmalloc(items_size(tag)); memcpy(tag->items, bulk.items, - (tag->numOfItems - 1) * sizeof(tag->items[0])); + items_size(tag) - sizeof(struct tag_item)); } tag->items[i] = tag_pool_get_item(type, p, len); -- cgit v1.2.3 From f0a68935f9da6a77cd4688fc5836b2ce8f058041 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 5 Sep 2008 00:10:48 -0700 Subject: tag: lock all accesses to tag_pool The tag pool is a shared global resource that is infrequently modified. However, it can occasionally be modified by several threads, especially by the metadata_pipe for streaming metadata (both reading/writing). The bulk tag_item pool is NOT locked as currently only the update thread uses it. --- src/tag.c | 16 ++++++++++------ src/tag_pool.c | 2 ++ src/tag_pool.h | 3 +++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/tag.c b/src/tag.c index dc2bab2d6..8208eb965 100644 --- a/src/tag.c +++ b/src/tag.c @@ -267,8 +267,9 @@ static void deleteItem(struct mpd_tag *tag, int idx) assert(idx < tag->numOfItems); tag->numOfItems--; + pthread_mutex_lock(&tag_pool_lock); tag_pool_put_item(tag->items[idx]); - /* free(tag->items[idx].value); */ + pthread_mutex_unlock(&tag_pool_lock); if (tag->numOfItems - idx > 0) { memmove(tag->items + idx, tag->items + idx + 1, @@ -300,10 +301,10 @@ static void clearMpdTag(struct mpd_tag *tag) { int i; - for (i = 0; i < tag->numOfItems; i++) { - /* free(tag->items[i].value); */ + pthread_mutex_lock(&tag_pool_lock); + for (i = 0; i < tag->numOfItems; i++) tag_pool_put_item(tag->items[i]); - } + pthread_mutex_unlock(&tag_pool_lock); if (tag->items == bulk.items) { #ifndef NDEBUG @@ -340,9 +341,10 @@ struct mpd_tag *tag_dup(const struct mpd_tag *tag) ret->numOfItems = tag->numOfItems; ret->items = ret->numOfItems > 0 ? xmalloc(items_size(tag)) : NULL; - for (i = 0; i < tag->numOfItems; i++) { + pthread_mutex_lock(&tag_pool_lock); + for (i = 0; i < tag->numOfItems; i++) ret->items[i] = tag_pool_dup_item(tag->items[i]); - } + pthread_mutex_unlock(&tag_pool_lock); return ret; } @@ -451,7 +453,9 @@ static void appendToTagItems(struct mpd_tag *tag, enum tag_type type, items_size(tag) - sizeof(struct tag_item)); } + pthread_mutex_lock(&tag_pool_lock); tag->items[i] = tag_pool_get_item(type, p, len); + pthread_mutex_unlock(&tag_pool_lock); if (p != value) free(deconst_ptr(p)); diff --git a/src/tag_pool.c b/src/tag_pool.c index 89efef1fc..d227a2988 100644 --- a/src/tag_pool.c +++ b/src/tag_pool.c @@ -19,6 +19,8 @@ #include "tag_pool.h" #include "utils.h" +pthread_mutex_t tag_pool_lock = PTHREAD_MUTEX_INITIALIZER; + #define NUM_SLOTS 4096 struct slot { diff --git a/src/tag_pool.h b/src/tag_pool.h index e19b2f4b4..d837d4446 100644 --- a/src/tag_pool.h +++ b/src/tag_pool.h @@ -20,6 +20,9 @@ #define TAG_POOL_H #include "tag.h" +#include "os_compat.h" + +extern pthread_mutex_t tag_pool_lock; struct tag_item; -- cgit v1.2.3