From f57ff3aa41f870dae04ab0046c5b1b8d21eba9c1 Mon Sep 17 00:00:00 2001 Message-Id: From: Mark Wooding Date: Tue, 24 Nov 2009 16:57:19 +0000 Subject: [PATCH] Correct playlist unique ID construction Organization: Straylight/Edgeware From: Richard Kettlewell --- disobedience/playlists.c | 39 ++++++++++++++++-------------------- disobedience/queue-generic.c | 2 ++ 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/disobedience/playlists.c b/disobedience/playlists.c index 37bb0ea..92772d7 100644 --- a/disobedience/playlists.c +++ b/disobedience/playlists.c @@ -756,7 +756,7 @@ static void playlists_editor_received_tracks(void *v, * so we add a serial number to the start. */ int *serialp = hash_find(h, vec[n]), serial = serialp ? *serialp : 0; byte_xasprintf((char **)&q->id, "%d-%s", serial++, vec[n]); - hash_add(h, vec[0], &serial, HASH_INSERT_OR_REPLACE); + hash_add(h, vec[n], &serial, HASH_INSERT_OR_REPLACE); *qq = q; qq = &q->next; } @@ -767,6 +767,16 @@ static void playlists_editor_received_tracks(void *v, /* Playlist mutation -------------------------------------------------------- */ /** @brief State structure for guarded playlist modification + * + * To safely move, insert or delete rows we must: + * - take a lock + * - fetch the playlist + * - verify it's not changed + * - update the playlist contents + * - store the playlist + * - release the lock + * + * The playlist_modify_ functions do just that. * * To kick things off create one of these and disorder_eclient_playlist_lock() * with playlist_modify_locked() as its callback. @c modify will be called; it @@ -870,15 +880,6 @@ static void playlist_drop(struct queuelike attribute((unused)) *ql, mod->tracks = tracks; mod->ids = ids; mod->after_me = after_me; - /* To safely move or insert rows we must: - * - take a lock - * - fetch the playlist - * - verify it's not changed - * - update the playlist contents - * - store the playlist - * - release the lock - * - */ disorder_eclient_playlist_lock(client, playlist_modify_locked, mod->playlist, mod); } @@ -887,6 +888,9 @@ static void playlist_drop_modify(struct playlist_modify_data *mod, int nvec, char **vec) { char **newvec; int nnewvec; + + fprintf(stderr, "after_me = %s\n", + mod->after_me ? mod->after_me->track : "NULL"); if(mod->ids) { /* This is a rearrangement */ /* TODO what if it's a drag from the queue? */ @@ -902,6 +906,8 @@ static void playlist_drop_modify(struct playlist_modify_data *mod, ++ins; } } + fprintf(stderr, "ins = %d = %s\n", + ins, ins < nvec ? vec[ins] : "NULL"); nnewvec = nvec + mod->ntracks; newvec = xcalloc(nnewvec, sizeof (char *)); memcpy(newvec, vec, @@ -954,20 +960,9 @@ static int playlist_remove_sensitive(void attribute((unused)) *extra) { /** @brief Called to play the selected playlist */ static void playlist_remove_activate(GtkMenuItem attribute((unused)) *menuitem, gpointer attribute((unused)) user_data) { + /* TODO backspace should work too */ if(!playlist_picker_selected) return; - /* To safely remove rows we must: - * - take a lock - * - fetch the playlist - * - verify it's not changed - * - delete the selected rows from the retrieved version - * - store the playlist - * - release the lock - * - * In addition we careful check that the selected playlist hasn't changed - * underfoot, and avoid leaving the playlist locked if we bail out at any - * point. - */ struct playlist_modify_data *mod = xmalloc(sizeof *mod); mod->playlist = playlist_picker_selected; diff --git a/disobedience/queue-generic.c b/disobedience/queue-generic.c index 05feb5b..e5f76a9 100644 --- a/disobedience/queue-generic.c +++ b/disobedience/queue-generic.c @@ -426,6 +426,8 @@ void ql_new_queue(struct queuelike *ql, nit = gtk_tree_model_iter_next(GTK_TREE_MODEL(ql->store), next); ++searches; } + /* Note that this assertion will fail in the face of duplicate IDs. + * q->id really does need to be unique. */ assert(nit); gtk_list_store_swap(ql->store, iter, next); *iter = *next; -- [mdw]