Revert the queue implementation and commands

It's too ugly and broken (both technically and usability-wise)
to be worth supporting in any stable release.

In one sentence: The queue is a very crippled version of the
playlist that takes precedence over the normal playlist.

How is it crippled?

* The "queueid" command only allows the queuing of songs
ALREADY IN THE PLAYLIST!  This promotes having the entire mpd
database of songs in the playlist, which is a stupid practice
to begin with.

* It doesn't allow for meaningful rearranging and movement
of songs within the queue.  To move a song, you'd need to
dequeue and requeue it (and other songs on the list).
Why?  The playlist already allows _all_ these features
and shows everything a client needs to know about the ordering
of songs in a _single_ command!

* Random was a stupid idea to begin with and unfortunately
we're stuck supporting it since we've always had it.  Users
should learn to use "shuffle" instead and not look at their
playlists.  Implementing queue because we have the problem of
random is just a bandage fix and digging ourselves a new hole.

This protocol addition was never in a stable release of mpd, so
reverting it will only break things for people following trunk;
which I'm not too worried about.  I am however worried about
long-term support of this misfeature, so I'm removing it.

Additionally, there are other points:

* It's trivially DoS-able:

(while true; do echo queueid $song_id; done) | nc $MPD_HOST $MPD_PORT

The above commands would cause the queue to become infinitely
expanding, taking up all available memory in the system.  The
mpd playlist was implemented as an array with a fixed (but
configurable) size limit for this reason.

* It's not backwards-compatible.  All clients would require
upgrades (and additional complexity) to even know what the
next song in the playlist is.  mpd is a shared architecture,
and we should not violate the principle of least astonishment
here.

This removes the following commands:
queueid, dequeue, queueinfo

Additionally, the status field of "playlistqueue: " is removed
from the status command.

While this DoS is trivial to fix, the design is simply too
broken to ever support in a real release.

The overloading of the "addid" command and the allowing of
negative numbers to be used as offsets is far more flexible.

This improved "addid" is completely backwards-compatible with
all clients, and does not require clients to have UI changes or
run additional commands to display the queue.

git-svn-id: https://svn.musicpd.org/mpd/trunk@7155 09075e82-0dd4-0310-85a5-a0d7c8717e4f

5 files changed, 1 insertions, 259 deletions

diff --git a/src/command.c b/src/command.c
index c621702b..eddb49af 100644
--- a/src/command.c
+++ b/src/command.c
@@ -93,9 +93,6 @@
 #define COMMAND_PLAYLISTSEARCH	"playlistsearch"
 #define COMMAND_PLAYLISTMOVE	"playlistmove"
 #define COMMAND_PLAYLISTDELETE	"playlistdelete"
-#define COMMAND_QUEUEID 	"queueid"
-#define COMMAND_DEQUEUE 	"dequeue"
-#define COMMAND_QUEUEINFO 	"queueinfo"
 #define COMMAND_TAGTYPES	"tagtypes"
 #define COMMAND_COUNT		"count"
 #define COMMAND_RENAME		"rename"
@@ -105,7 +102,6 @@
 #define COMMAND_STATUS_REPEAT           "repeat"
 #define COMMAND_STATUS_RANDOM           "random"
 #define COMMAND_STATUS_PLAYLIST         "playlist"
-#define COMMAND_STATUS_PLAYLIST_QUEUE  "playlistqueue"
 #define COMMAND_STATUS_PLAYLIST_LENGTH  "playlistlength"
 #define COMMAND_STATUS_SONG             "song"
 #define COMMAND_STATUS_SONGID           "songid"
@@ -320,8 +316,6 @@ static int commandStatus(int fd, int *permission, int argc, char *argv[])
 		 getPlaylistRandomStatus());
 	fdprintf(fd, "%s: %li\n", COMMAND_STATUS_PLAYLIST,
 		 getPlaylistVersion());
-	fdprintf(fd, "%s: %li\n", COMMAND_STATUS_PLAYLIST_QUEUE,
-		 getPlaylistQueueVersion());
 	fdprintf(fd, "%s: %i\n", COMMAND_STATUS_PLAYLIST_LENGTH,
 		 getPlaylistLength());
 	fdprintf(fd, "%s: %i\n", COMMAND_STATUS_CROSSFADE,
@@ -642,47 +636,6 @@ static int handlePlaylistMove(int fd, int *permission, int argc, char *argv[])
 	return moveSongInStoredPlaylistByPath(fd, playlist, from, to);
 }
 
-static int handleQueueInfo(int fd, int *permission, int argc, char *argv[])
-{
-	return playlistQueueInfo(fd);
-}
-
-static int handleQueueId(int fd, int *permission, int argc, char *argv[])
-{
-	int id, position = -1;
-	char *test;
-
-	id = strtol(argv[1], &test, 10);
-	if (*test != '\0') {
-		commandError(fd, ACK_ERROR_ARG,
-			     "\"%s\" is not a integer", argv[1]);
-		return -1;
-	}
-	if (argc == 3) {
-		position = strtol(argv[2], &test, 10);
-		if (*test != '\0') {
-			commandError(fd, ACK_ERROR_ARG,
-		        	     "\"%s\" is not a integer", argv[2]);
-			return -1;
-		}
-	}
-	return addToPlaylistQueueById(fd, id, position);
-}
-
-static int handleDequeue(int fd, int *permission, int argc, char *argv[])
-{
-	int pos;
-	char *test;
-
-	pos = strtol(argv[1], &test, 10);
-	if (*test != '\0') {
-		commandError(fd, ACK_ERROR_ARG,
-			     "\"%s\" is not a integer", argv[1]);
-		return -1;
-	}
-	return deleteFromPlaylistQueue(fd, pos); 
-}
-
 static int listHandleUpdate(int fd,
 			    int *permission,
 			    int argc,
@@ -1085,9 +1038,6 @@ void initCommands(void)
 	addCommand(COMMAND_PLAYLISTSEARCH,   PERMISSION_READ,    2,   -1,  handlePlaylistSearch,       NULL);
 	addCommand(COMMAND_PLAYLISTMOVE,     PERMISSION_CONTROL, 3,   3,   handlePlaylistMove,         NULL);
 	addCommand(COMMAND_PLAYLISTDELETE,   PERMISSION_CONTROL, 2,   2,   handlePlaylistDelete,       NULL);
-	addCommand(COMMAND_QUEUEINFO,        PERMISSION_CONTROL, 0,   0,   handleQueueInfo,            NULL);
-	addCommand(COMMAND_QUEUEID,          PERMISSION_CONTROL, 1,   2,   handleQueueId,              NULL);
-	addCommand(COMMAND_DEQUEUE,          PERMISSION_CONTROL, 1,   1,   handleDequeue,              NULL);
 	addCommand(COMMAND_TAGTYPES,         PERMISSION_READ,    0,   0,   handleTagTypes,             NULL);
 	addCommand(COMMAND_COUNT,            PERMISSION_READ,    2,   -1,  handleCount,                NULL);
 	addCommand(COMMAND_RENAME,           PERMISSION_CONTROL, 2,   2,   handleRename,               NULL);
diff --git a/src/list.c b/src/list.c
index b360fe65..6a54d8a5 100644
--- a/src/list.c
+++ b/src/list.c
@@ -282,21 +282,6 @@ int findInList(List * list, char *key, void **data)
 	return 0;
 }
 
-ListNode *getNodeByPosition(List *list, int pos)
-{
-	ListNode *tmpNode;
-
-	assert(list != NULL);
-	if (pos < 0 || pos >= list->numberOfNodes)
-		return NULL;
-
-	tmpNode = list->firstNode;
-	while (pos-- > 0) 
-		tmpNode = tmpNode->nextNode;
-
-	return tmpNode;
-}
-
 int deleteFromList(List * list, char *key)
 {
 	ListNode *tmpNode;
diff --git a/src/list.h b/src/list.h
index 2c4d423e..62c40aef 100644
--- a/src/list.h
+++ b/src/list.h
@@ -99,12 +99,6 @@ int findInList(List * list, char *key, void **data);
 	the info would be found */
 int findNodeInList(List * list, char *key, ListNode ** node, int *pos);
 
-/*
- * returns ListNode at position _pos_ from first node. If no ListNode exists
- * at position _pos_ returns NULL
- */
-ListNode *getNodeByPosition(List *list, int pos);
-
 /* frees memory malloc'd for list and its nodes
  *  _list_ -> List to be free'd
  */
diff --git a/src/playlist.c b/src/playlist.c
index e67fb943..a01fc806 100644
--- a/src/playlist.c
+++ b/src/playlist.c
@@ -66,12 +66,9 @@ static int playlist_noGoToNext;
 
 int playlist_saveAbsolutePaths = DEFAULT_PLAYLIST_SAVE_ABSOLUTE_PATHS;
 
-static List *playlistQueue;
-
 static void swapOrder(int a, int b);
 static int playPlaylistOrderNumber(int fd, int orderNum);
 static void randomizeOrder(int start, int end);
-static void clearPlayerQueue(void);
 
 static void incrPlaylistVersion(void)
 {
@@ -99,14 +96,6 @@ void playlistVersionChange(void)
 	incrPlaylistVersion();
 }
 
-static void incrPlaylistQueueVersion(void)
-{
-	static unsigned long max = ((mpd_uint32) 1 << 31) - 1;
-	playlist.queueversion++;
-	if (playlist.queueversion >= max) 
-		playlist.queueversion = 1;
-}
-
 static void incrPlaylistCurrent(void)
 {
 	if (playlist.current < 0)
@@ -130,7 +119,6 @@ void initPlaylist(void)
 	playlist.length = 0;
 	playlist.repeat = 0;
 	playlist.version = 1;
-	playlist.queueversion = 1;
 	playlist.random = 0;
 	playlist.queued = -1;
 	playlist.current = -1;
@@ -165,8 +153,6 @@ void initPlaylist(void)
 	for (i = 0; i < playlist_max_length * PLAYLIST_HASH_MULT; i++) {
 		playlist.idToPosition[i] = -1;
 	}
-	
-	playlistQueue = makeList(DEFAULT_FREE_DATA_FUNC, 0);
 }
 
 static int getNextId(void)
@@ -212,7 +198,6 @@ int clearPlaylist(int fd)
 
 	if (stopPlaylist(fd) < 0)
 		return -1;
-	clearPlaylistQueue();
 
 	for (i = 0; i < playlist.length; i++) {
 		if (playlist.songs[i]->type == SONG_TYPE_URL) {
@@ -498,29 +483,7 @@ static void queueNextSongInPlaylist(void)
 {
 	char path_max_tmp[MPD_PATH_MAX];
 
-	if (playlistQueue->numberOfNodes != 0) {
-		int i;
-		/* we need to find where in order[] is first song from queue */
-		for (i=0;i < playlist.length; i++)
-			if (playlist.order[i] == playlist.
-			    idToPosition[*(int *)playlistQueue->
-			    firstNode->data])
-				break;
-		clearPlayerQueue();
-		playlist.queued = i;
-		DEBUG("playlist: queue song %i:\"%s\"\n",
-		      playlist.queued,
-		      get_song_url(path_max_tmp,
-		                   playlist.
-				   songs[playlist.order[playlist.queued]]));
-		
-		if (queueSong(playlist.songs[playlist.order[playlist.queued]]) <
-		    0) {
-			playlist.queued = -1;
-			playlist_queueError = 1;
-		}
-	} else if (playlist.current < playlist.length - 1) {
-		clearPlayerQueue();
+	if (playlist.current < playlist.length - 1) {
 		playlist.queued = playlist.current + 1;
 		DEBUG("playlist: queue song %i:\"%s\"\n",
 		      playlist.queued,
@@ -536,7 +499,6 @@ static void queueNextSongInPlaylist(void)
 		if (playlist.length > 1 && playlist.random) {
 			randomizeOrder(0, playlist.length - 1);
 		}
-		clearPlayerQueue();
 		playlist.queued = 0;
 		DEBUG("playlist: queue song %i:\"%s\"\n",
 		      playlist.queued,
@@ -565,9 +527,6 @@ static void syncPlaylistWithQueue(int queue)
 		if (playlist.queued >= 0) {
 			DEBUG("playlist: now playing queued song\n");
 			playlist.current = playlist.queued;
-			if (playlistQueue->numberOfNodes > 0) {
-				deleteFromPlaylistQueueInternal(0);
-			}
 		}
 		playlist.queued = -1;
 		if (queue)
@@ -778,29 +737,12 @@ int deleteFromPlaylist(int fd, int song)
 {
 	int i;
 	int songOrder;
-	ListNode *qItem;
 
 	if (song < 0 || song >= playlist.length) {
 		commandError(fd, ACK_ERROR_NO_EXIST,
 			     "song doesn't exist: \"%i\"", song);
 		return -1;
 	}
-	
-	/* we need to clear song from queue */
-	i = 0;
-	qItem = playlistQueue->firstNode;
-	while (qItem) {
-		if (playlist.idToPosition[*(int *)qItem->data] == 
-				song) {
-                
-			qItem = qItem->nextNode;
-			deleteFromPlaylistQueueInternal(i); 
-			/* can be queued multiple times */
-			continue;
-		}
-		i++;
-		qItem = qItem->nextNode;
-	}
 
 	if (playlist_state == PLAYLIST_STATE_PLAY) {
 		if (playlist.queued >= 0
@@ -920,28 +862,9 @@ static int playPlaylistOrderNumber(int fd, int orderNum)
 
 	playlist.current = orderNum;
 
-	/* are we playing from queue ? */
-	if (playlistQueue->numberOfNodes > 0 &&
-	    playlist.idToPosition[*(int *)playlistQueue->
-	    firstNode->data] == playlist.order[orderNum]) {
-		deleteFromPlaylistQueueInternal(0);
-		queueNextSongInPlaylist();
-	}
-
 	return 0;
 }
 
-int playNextPlaylistQueue(int fd, int stopOnError)
-{
-	int ret;
-	if (playlistQueue->numberOfNodes == 0)
-		return -1;
-
-	ret = playPlaylistById(fd, *(int *)playlistQueue->firstNode->data, 
-			       stopOnError);
-	return ret;
-}
-
 int playPlaylist(int fd, int song, int stopOnError)
 {
 	int i = song;
@@ -955,12 +878,6 @@ int playPlaylist(int fd, int song, int stopOnError)
 		if (playlist_state == PLAYLIST_STATE_PLAY) {
 			return playerSetPause(fd, 0);
 		}
-
-		if (playlist_state != PLAYLIST_STATE_STOP &&
-		    playNextPlaylistQueue(fd, stopOnError) == 0) {
-			return 0;
-		}
-
 		if (playlist.current >= 0 && playlist.current < playlist.length) {
 			i = playlist.current;
 		} else {
@@ -1069,9 +986,6 @@ int nextSongInPlaylist(int fd)
 
 	playlist_stopOnError = 0;
 
-	if (playNextPlaylistQueue(fd, 0) == 0)
-		return 0;
-
 	if (playlist.current < playlist.length - 1) {
 		return playPlaylistOrderNumber(fd, playlist.current + 1);
 	} else if (playlist.length && playlist.repeat) {
@@ -1467,11 +1381,6 @@ unsigned long getPlaylistVersion(void)
 	return playlist.version;
 }
 
-unsigned long getPlaylistQueueVersion(void)
-{
-	return playlist.queueversion;
-}
-
 int getPlaylistLength(void)
 {
 	return playlist.length;
@@ -1622,87 +1531,6 @@ void findSongsInPlaylist(int fd, int numItems, LocateTagItem * items)
 	}
 }
 
-void clearPlaylistQueue(void)
-{
-	freeList(playlistQueue);
-	playlistQueue = makeList(DEFAULT_FREE_DATA_FUNC, 0);
-        incrPlaylistQueueVersion();
-}
-
-int addToPlaylistQueueById(int fd, int song, int toPosition)
-{
-	int pos, *data;
-	ListNode *prevItem;
-
-	pos = playlist.idToPosition[song];
-	if (pos < 0 || pos >= playlist.length) {
-		commandError(fd, ACK_ERROR_NO_EXIST,
-			     "song doesn't exist: \"%i\"", song);
-		return -1;
-	}
-	if (toPosition < -1 || toPosition > playlistQueue->numberOfNodes) {
-		commandError(fd, ACK_ERROR_ARG,
-			     "queue position out of range: \"%i\"", toPosition);
-		return -1;
-	}
-	data = xmalloc(sizeof(int));
-	*data = song;
-	if (toPosition == -1) {
-		insertInList(playlistQueue, (char *)1, data);
-	} else {
-		prevItem = getNodeByPosition(playlistQueue, toPosition);
-		if (prevItem == NULL) {
-			insertInList(playlistQueue, (char *)1, data);
-		} else 
-			insertInListBeforeNode(playlistQueue, prevItem, -1,
-					       (char*) 1, data); 
-	}
-
-        if (playlistQueue->numberOfNodes == 1 || toPosition == 0)
-            queueNextSongInPlaylist();
-        incrPlaylistQueueVersion();
-	return 0;
-}
-
-int deleteFromPlaylistQueue(int fd, int song)
-{
-	if (song < 0 || song >= playlistQueue->numberOfNodes) {
-		commandError(fd, ACK_ERROR_NO_EXIST,
-			     "song doesn't exist: \"%i\"", song);
-		return -1;
-	}
-
-	return deleteFromPlaylistQueueInternal(song);
-}
-
-int deleteFromPlaylistQueueInternal(int song)
-{
-	ListNode *delItem;
-
-	delItem = getNodeByPosition(playlistQueue, song);
-	if (delItem == NULL)
-		return -1;
-
-	deleteNodeFromList(playlistQueue, delItem);
-	if (song == 0) 
-		queueNextSongInPlaylist();
-
-        incrPlaylistQueueVersion();
-	return 0;
-}
-
-int playlistQueueInfo(int fd)
-{
-	ListNode *cur = playlistQueue->firstNode;
-	int no = 0;
-	while (cur) {
-		printSongInfo(fd, playlist.songs[playlist.idToPosition[*(int *)cur->data]]);
-		fdprintf(fd, "Pos: %i\nId: %i\n", no++, *(int *)cur->data);
-		cur = cur->nextNode;
-	}
-	return 0;
-}
-
 /*
  * Not supporting '/' was done out of laziness, and we should really
  * strive to support it in the future.
diff --git a/src/playlist.h b/src/playlist.h
index 732ea814..76ef19f3 100644
--- a/src/playlist.h
+++ b/src/playlist.h
@@ -40,7 +40,6 @@ typedef struct _Playlist {
 	int repeat;
 	int random;
 	mpd_uint32 version;
-	mpd_uint32 queueversion;
 } Playlist;
 
 extern int playlist_saveAbsolutePaths;
@@ -79,8 +78,6 @@ int stopPlaylist(int fd);
 
 int playPlaylist(int fd, int song, int stopOnError);
 
-int playNextPlaylistQueue(int fd, int stopOnError);
-
 int playPlaylistById(int fd, int song, int stopOnError);
 
 int nextSongInPlaylist(int fd);
@@ -125,8 +122,6 @@ int getPlaylistLength(void);
 
 unsigned long getPlaylistVersion(void);
 
-unsigned long getPlaylistQueueVersion(void);
-
 void playPlaylistIfPlayerStopped(void);
 
 int seekSongInPlaylist(int fd, int song, float time);
@@ -145,16 +140,6 @@ void searchForSongsInPlaylist(int fd, int numItems, LocateTagItem * items);
 
 void findSongsInPlaylist(int fd, int numItems, LocateTagItem * items);
 
-void clearPlaylistQueue(void);
-
-int addToPlaylistQueueById(int fd, int song, int toPosition);
-
-int deleteFromPlaylistQueue(int fd, int song);
-
-int deleteFromPlaylistQueueInternal(int song);
-
-int playlistQueueInfo(int fd);
-
 int valid_playlist_name(int err_fd, const char *utf8path);
 
 #endif