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.
This commit is contained in:
Eric Wong 2008-09-06 15:31:55 +02:00 committed by Max Kellermann
parent 6146d4f5bb
commit 092bdf3d32
2 changed files with 22 additions and 14 deletions

View File

@ -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;

View File

@ -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);