Client: move code to class CommandListBuilder

Makefile.am

@@ -240,6 +240,7 @@ src_mpd_SOURCES = \
 	src/UpdateContainer.cxx src/UpdateContainer.hxx \
 	src/UpdateInternal.hxx \
 	src/UpdateRemove.cxx src/UpdateRemove.hxx \
+	src/CommandListBuilder.cxx src/CommandListBuilder.hxx \
 	src/Client.cxx src/Client.hxx \
 	src/ClientInternal.hxx \
 	src/ClientEvent.cxx \

src/ClientInternal.hxx

@@ -22,6 +22,7 @@
 
 #include "Client.hxx"
 #include "ClientMessage.hxx"
+#include "CommandListBuilder.hxx"
 #include "command.h"
 
 #include <set>
@@ -63,9 +64,8 @@ public:
 	 */
 	GTimer *last_activity;
 
-	GSList *cmd_list;	/* for when in list mode */
+	CommandListBuilder cmd_list;
-	int cmd_list_OK;	/* print OK after each command execution */
+
-	size_t cmd_list_size;	/* mem cmd_list consumes */
 	GQueue *deferred_send;	/* for output if client is slow */
 	size_t deferred_bytes;	/* mem deferred_send consumes */
 	unsigned int num;	/* client number */
@@ -135,21 +135,6 @@ client_list_remove(Client *client);
 void
 client_close(Client *client);
 
-static inline void
-new_cmd_list_ptr(Client *client, const char *s)
-{
-	client->cmd_list = g_slist_prepend(client->cmd_list, g_strdup(s));
-}
-
-static inline void
-free_cmd_list(GSList *list)
-{
-	for (GSList *tmp = list; tmp != NULL; tmp = g_slist_next(tmp))
-		g_free(tmp->data);
-
-	g_slist_free(list);
-}
-
 void
 client_set_expired(Client *client);
 

src/ClientNew.cxx

@@ -52,7 +52,6 @@ Client::Client(struct player_control *_player_control,
 	 permission(getDefaultPermissions()),
 	 uid(_uid),
 	 last_activity(g_timer_new()),
-	 cmd_list(nullptr), cmd_list_OK(-1), cmd_list_size(0),
 	 deferred_send(g_queue_new()), deferred_bytes(0),
 	 num(_num),
 	 send_buf_used(0),
@@ -86,9 +85,6 @@ Client::~Client()
 {
 	g_timer_destroy(last_activity);
 
-	if (cmd_list != nullptr)
-		free_cmd_list(cmd_list);
-
 	g_queue_foreach(deferred_send, deferred_buffer_free, NULL);
 	g_queue_free(deferred_send);
 

src/ClientProcess.cxx

@@ -76,19 +76,16 @@ client_process_line(Client *client, char *line)
 		return COMMAND_RETURN_CLOSE;
 	}
 
-	if (client->cmd_list_OK >= 0) {
+	if (client->cmd_list.IsActive()) {
 		if (strcmp(line, CLIENT_LIST_MODE_END) == 0) {
 			g_debug("[%u] process command list",
 				client->num);
 
-			/* for scalability reasons, we have prepended
+			auto cmd_list = client->cmd_list.Commit();
-			   each new command; now we have to reverse it
-			   to restore the correct order */
-			client->cmd_list = g_slist_reverse(client->cmd_list);
 
 			ret = client_process_command_list(client,
-							  client->cmd_list_OK,
+							  client->cmd_list.IsOKMode(),
-							  client->cmd_list);
+							  cmd_list);
 			g_debug("[%u] process command "
 				"list returned %i", client->num, ret);
 
@@ -100,31 +97,24 @@ client_process_line(Client *client, char *line)
 				command_success(client);
 
 			client_write_output(client);
-			free_cmd_list(client->cmd_list);
+			client->cmd_list.Reset();
-			client->cmd_list = NULL;
-			client->cmd_list_OK = -1;
 		} else {
-			size_t len = strlen(line) + 1;
+			if (!client->cmd_list.Add(line)) {
-			client->cmd_list_size += len;
+				g_warning("[%u] command list size "
-			if (client->cmd_list_size >
-			    client_max_command_list_size) {
-				g_warning("[%u] command list size (%lu) "
 					  "is larger than the max (%lu)",
 					  client->num,
-					  (unsigned long)client->cmd_list_size,
 					  (unsigned long)client_max_command_list_size);
 				return COMMAND_RETURN_CLOSE;
 			}
 
-			new_cmd_list_ptr(client, line);
 			ret = COMMAND_RETURN_OK;
 		}
 	} else {
 		if (strcmp(line, CLIENT_LIST_MODE_BEGIN) == 0) {
-			client->cmd_list_OK = 0;
+			client->cmd_list.Begin(false);
 			ret = COMMAND_RETURN_OK;
 		} else if (strcmp(line, CLIENT_LIST_OK_MODE_BEGIN) == 0) {
-			client->cmd_list_OK = 1;
+			client->cmd_list.Begin(true);
 			ret = COMMAND_RETURN_OK;
 		} else {
 			g_debug("[%u] process command \"%s\"",

src/CommandListBuilder.cxx

@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2003-2013 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "CommandListBuilder.hxx"
+#include "ClientInternal.hxx"
+
+#include <string.h>
+
+void
+CommandListBuilder::Reset()
+{
+	for (GSList *tmp = cmd_list; tmp != NULL; tmp = g_slist_next(tmp))
+		g_free(tmp->data);
+
+	g_slist_free(cmd_list);
+
+	cmd_list = nullptr;
+	cmd_list_OK = -1;
+}
+
+bool
+CommandListBuilder::Add(const char *cmd)
+{
+	size_t len = strlen(cmd) + 1;
+	cmd_list_size += len;
+	if (cmd_list_size > client_max_command_list_size)
+		return false;
+
+	cmd_list = g_slist_prepend(cmd_list, g_strdup(cmd));
+	return true;
+}

src/CommandListBuilder.hxx

@@ -0,0 +1,100 @@
+/*
+ * Copyright (C) 2003-2013 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_COMMAND_LIST_BUILDER_HXX
+#define MPD_COMMAND_LIST_BUILDER_HXX
+
+#include <glib.h>
+#include <assert.h>
+
+class CommandListBuilder {
+	/**
+	 * for when in list mode
+	 */
+	GSList *cmd_list;
+
+	/**
+	 * print OK after each command execution
+	 */
+	int cmd_list_OK;
+
+	/**
+	 * mem cmd_list consumes
+	 */
+	size_t cmd_list_size;
+
+public:
+	CommandListBuilder()
+		:cmd_list(nullptr), cmd_list_OK(-1), cmd_list_size(0) {}
+	~CommandListBuilder() {
+		Reset();
+	}
+
+	/**
+	 * Is a command list currently being built?
+	 */
+	bool IsActive() const {
+		assert(cmd_list_OK >= -1 && cmd_list_OK <= 1);
+
+		return cmd_list_OK >= 0;
+	}
+
+	/**
+	 * Is the object in "list_OK" mode?
+	 */
+	bool IsOKMode() const {
+		assert(IsActive());
+
+		return (bool)cmd_list_OK;
+	}
+
+	/**
+	 * Reset the object: delete the list and clear the mode.
+	 */
+	void Reset();
+
+	/**
+	 * Begin building a command list.
+	 */
+	void Begin(bool ok) {
+		assert(cmd_list == nullptr);
+		assert(cmd_list_OK == -1);
+
+		cmd_list_OK = (int)ok;
+	}
+
+	/**
+	 * @return false if the list is full
+	 */
+	bool Add(const char *cmd);
+
+	/**
+	 * Finishes the list and returns it.
+	 */
+	GSList *Commit() {
+		assert(IsActive());
+
+		/* for scalability reasons, we have prepended each new
+		   command; now we have to reverse it to restore the
+		   correct order */
+		return cmd_list = g_slist_reverse(cmd_list);
+	}
+};
+
+#endif