From 6e8200227f617b7604a01f372460661d8ec5da32 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
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(-)

(limited to 'src/tag.c')

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 <normalperson@yhbt.net>
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 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

(limited to 'src/tag.c')

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));
-- 
cgit v1.2.3