Max Kellermann <[EMAIL PROTECTED]> wrote:
> Max Kellermann (4):
> const pointers
> tag: fix the shout and oggflac plugins
> oggflac: fix GCC warnings
> tag: optimize tag_dup(), copy item references
>
> audioOutputs/audioOutput_shout.c | 16 +++++++-------
> inputPlugins/oggflac_plugin.c | 24 +++++++++++----------
> tag.c | 12 ++++++----
> tag.h | 8 +++----
> tag_id3.c | 2 -
> tag_id3.h | 2 -
> tag_pool.c | 43
> +++++++++++++++++++++++++++++++++------
> tag_pool.h | 2 +
> 8 files changed, 73 insertions(+), 36 deletions(-)
I'm hitting a segfault when running update, the patch below fixes
things for me.
However I'm thinking the tag_begin_add/tag_end_add() interface is too
fragile and error prone to be worth exposing.
Perhaps make both tag_begin_add and tag_end_add (if busy) get called
implicitly whenever tag_new() is called.
>From 1b48920780d016ac4a93820dd938e51757cc532d Mon Sep 17 00:00:00 2001
From: Eric Wong <[EMAIL PROTECTED]>
Date: Wed, 3 Sep 2008 02:14:52 -0700
Subject: [PATCH] tag: fix segfault on update
clearMpdTag could be called on a tag that was still in a
tag_begin_add transaction before tag_end_add is called. This
was causing free() to attempt to operate on bulk.items; which is
un-free()-able. Now instead we unmark the bulk.busy to avoid
committing the tags to the heap only to be immediately freed.
Additionally, we need to remember to call tag_end_add() when
a song is updated before we NULL song->tag to avoid tripping
an assertion the next time tag_begin_add() is called.
---
src/song.c | 1 +
src/tag.c | 35 +++++++++++++++++++++--------------
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/src/song.c b/src/song.c
index 8651a01..067ce44 100644
--- a/src/song.c
+++ b/src/song.c
@@ -202,6 +202,7 @@ static void insertSongIntoList(SongList * list, ListNode **
nextSongNode,
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;
diff --git a/src/tag.c b/src/tag.c
index d76ba5d..6e31a16 100644
--- a/src/tag.c
+++ b/src/tag.c
@@ -26,6 +26,19 @@
#include "tagTracker.h"
#include "song.h"
+/**
+ * Maximum number of items managed in the bulk list; if it is
+ * exceeded, we switch back to "normal" reallocation.
+ */
+#define BULK_MAX 64
+
+static struct {
+#ifndef NDEBUG
+ int busy;
+#endif
+ struct tag_item *items[BULK_MAX];
+} bulk;
+
const char *mpdTagItemKeys[TAG_NUM_OF_ITEM_TYPES] = {
"Artist",
"Album",
@@ -288,8 +301,15 @@ static void clearMpdTag(struct tag *tag)
tag_pool_put_item(tag->items[i]);
}
- if (tag->items)
+ if (tag->items == bulk.items) {
+#ifndef NDEBUG
+ assert(bulk.busy);
+ bulk.busy = 0;
+#endif
+ } else if (tag->items) {
free(tag->items);
+ }
+
tag->items = NULL;
tag->numOfItems = 0;
@@ -363,19 +383,6 @@ static inline const char *fix_utf8(const char *str, size_t
*length_r) {
return temp;
}
-/**
- * Maximum number of items managed in the bulk list; if it is
- * exceeded, we switch back to "normal" reallocation.
- */
-#define BULK_MAX 64
-
-static struct {
-#ifndef NDEBUG
- int busy;
-#endif
- struct tag_item *items[BULK_MAX];
-} bulk;
-
void tag_begin_add(struct tag *tag)
{
assert(!bulk.busy);
--
Eric Wong
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
