protocol/Ack: add exception class wrapping enum ack

src/command/CommandError.cxx

@@ -165,6 +165,8 @@ PrintError(Response &r, const std::exception &e)
 
 	try {
 		throw e;
+	} catch (const ProtocolError &pe) {
+		r.Error(pe.GetCode(), pe.what());
 	} catch (const PlaylistError &pe) {
 		r.Error(ToAck(pe.GetCode()), pe.what());
 	} catch (const std::system_error &) {

src/command/DatabaseCommands.cxx

@@ -69,8 +69,7 @@ handle_match(Client &client, Request args, Response &r, bool fold_case)
 {
 	RangeArg window;
 	if (args.size >= 2 && StringIsEqual(args[args.size - 2], "window")) {
-		if (!args.Parse(args.size - 1, window, r))
-			return CommandResult::ERROR;
+		window = args.ParseRange(args.size - 1);
 
 		args.pop_back();
 		args.pop_back();

src/command/OtherCommands.cxx

@@ -352,9 +352,7 @@ handle_rescan(Client &client, Request args, Response &r)
 CommandResult
 handle_setvol(Client &client, Request args, Response &r)
 {
-	unsigned level;
-	if (!args.Parse(0, level, r, 100))
-		return CommandResult::ERROR;
+	unsigned level = args.ParseUnsigned(0, 100);
 
 	if (!volume_level_change(client.partition.outputs, level)) {
 		r.Error(ACK_ERROR_SYSTEM, "problems setting volume");
@@ -367,9 +365,7 @@ handle_setvol(Client &client, Request args, Response &r)
 CommandResult
 handle_volume(Client &client, Request args, Response &r)
 {
-	int relative;
-	if (!args.Parse(0, relative, r,  -100, 100))
-		return CommandResult::ERROR;
+	int relative = args.ParseInt(0, -100, 100);
 
 	const int old_volume = volume_level_get(client.partition.outputs);
 	if (old_volume < 0) {

src/command/OutputCommands.cxx

@@ -31,9 +31,7 @@ CommandResult
 handle_enableoutput(Client &client, Request args, Response &r)
 {
 	assert(args.size == 1);
-	unsigned device;
-	if (!args.Parse(0, device, r))
-		return CommandResult::ERROR;
+	unsigned device = args.ParseUnsigned(0);
 
 	if (!audio_output_enable_index(client.partition.outputs, device)) {
 		r.Error(ACK_ERROR_NO_EXIST, "No such audio output");
@@ -47,9 +45,7 @@ CommandResult
 handle_disableoutput(Client &client, Request args, Response &r)
 {
 	assert(args.size == 1);
-	unsigned device;
-	if (!args.Parse(0, device, r))
-		return CommandResult::ERROR;
+	unsigned device = args.ParseUnsigned(0);
 
 	if (!audio_output_disable_index(client.partition.outputs, device)) {
 		r.Error(ACK_ERROR_NO_EXIST, "No such audio output");
@@ -63,9 +59,7 @@ CommandResult
 handle_toggleoutput(Client &client, Request args, Response &r)
 {
 	assert(args.size == 1);
-	unsigned device;
-	if (!args.Parse(0, device, r))
-		return CommandResult::ERROR;
+	unsigned device = args.ParseUnsigned(0);
 
 	if (!audio_output_toggle_index(client.partition.outputs, device)) {
 		r.Error(ACK_ERROR_NO_EXIST, "No such audio output");

src/command/PlayerCommands.cxx

@@ -57,23 +57,17 @@
 #define COMMAND_STATUS_UPDATING_DB	"updating_db"
 
 CommandResult
-handle_play(Client &client, Request args, Response &r)
+handle_play(Client &client, Request args, gcc_unused Response &r)
 {
-	int song = -1;
-	if (!args.ParseOptional(0, song, r))
-		return CommandResult::ERROR;
-
+	int song = args.ParseOptional(0, -1);
 	client.partition.PlayPosition(song);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_playid(Client &client, Request args, Response &r)
+handle_playid(Client &client, Request args, gcc_unused Response &r)
 {
-	int id = -1;
-	if (!args.ParseOptional(0, id, r))
-		return CommandResult::ERROR;
-
+	int id = args.ParseOptional(0, -1);
 	client.partition.PlayId(id);
 	return CommandResult::OK;
 }
@@ -93,13 +87,10 @@ handle_currentsong(Client &client, gcc_unused Request args, Response &r)
 }
 
 CommandResult
-handle_pause(Client &client, Request args, Response &r)
+handle_pause(Client &client, Request args, gcc_unused Response &r)
 {
 	if (!args.IsEmpty()) {
-		bool pause_flag;
-		if (!args.Parse(0, pause_flag, r))
-			return CommandResult::ERROR;
-
+		bool pause_flag = args.ParseBool(0);
 		client.player_control.LockSetPause(pause_flag);
 	} else
 		client.player_control.LockPause();
@@ -236,45 +227,33 @@ handle_previous(Client &client, gcc_unused Request args,
 }
 
 CommandResult
-handle_repeat(Client &client, Request args, Response &r)
+handle_repeat(Client &client, Request args, gcc_unused Response &r)
 {
-	bool status;
-	if (!args.Parse(0, status, r))
-		return CommandResult::ERROR;
-
+	bool status = args.ParseBool(0);
 	client.partition.SetRepeat(status);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_single(Client &client, Request args, Response &r)
+handle_single(Client &client, Request args, gcc_unused Response &r)
 {
-	bool status;
-	if (!args.Parse(0, status, r))
-		return CommandResult::ERROR;
-
+	bool status = args.ParseBool(0);
 	client.partition.SetSingle(status);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_consume(Client &client, Request args, Response &r)
+handle_consume(Client &client, Request args, gcc_unused Response &r)
 {
-	bool status;
-	if (!args.Parse(0, status, r))
-		return CommandResult::ERROR;
-
+	bool status = args.ParseBool(0);
 	client.partition.SetConsume(status);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_random(Client &client, Request args, Response &r)
+handle_random(Client &client, Request args, gcc_unused Response &r)
 {
-	bool status;
-	if (!args.Parse(0, status, r))
-		return CommandResult::ERROR;
-
+	bool status = args.ParseBool(0);
 	client.partition.SetRandom(status);
 	client.partition.outputs.SetReplayGainMode(replay_gain_get_real_mode(client.partition.GetRandom()));
 	return CommandResult::OK;
@@ -291,10 +270,8 @@ handle_clearerror(Client &client, gcc_unused Request args,
 CommandResult
 handle_seek(Client &client, Request args, Response &r)
 {
-	unsigned song;
-	SongTime seek_time;
-	if (!args.Parse(0, song, r) || !args.Parse(1, seek_time, r))
-		return CommandResult::ERROR;
+	unsigned song = args.ParseUnsigned(0);
+	SongTime seek_time = args.ParseSongTime(1);
 
 	Error error;
 	return client.partition.SeekSongPosition(song, seek_time, error)
@@ -305,12 +282,8 @@ handle_seek(Client &client, Request args, Response &r)
 CommandResult
 handle_seekid(Client &client, Request args, Response &r)
 {
-	unsigned id;
-	SongTime seek_time;
-	if (!args.Parse(0, id, r))
-		return CommandResult::ERROR;
-	if (!args.Parse(1, seek_time, r))
-		return CommandResult::ERROR;
+	unsigned id = args.ParseUnsigned(0);
+	SongTime seek_time = args.ParseSongTime(1);
 
 	Error error;
 	return client.partition.SeekSongId(id, seek_time, error)
@@ -323,9 +296,7 @@ handle_seekcur(Client &client, Request args, Response &r)
 {
 	const char *p = args.front();
 	bool relative = *p == '+' || *p == '-';
-	SignedSongTime seek_time;
-	if (!ParseCommandArg(r, seek_time, p))
-		return CommandResult::ERROR;
+	SignedSongTime seek_time = ParseCommandArgSignedSongTime(p);
 
 	Error error;
 	return client.partition.SeekCurrent(seek_time, relative, error)
@@ -334,36 +305,26 @@ handle_seekcur(Client &client, Request args, Response &r)
 }
 
 CommandResult
-handle_crossfade(Client &client, Request args, Response &r)
+handle_crossfade(Client &client, Request args, gcc_unused Response &r)
 {
-	unsigned xfade_time;
-	if (!args.Parse(0, xfade_time, r))
-		return CommandResult::ERROR;
-
+	unsigned xfade_time = args.ParseUnsigned(0);
 	client.player_control.SetCrossFade(xfade_time);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_mixrampdb(Client &client, Request args, Response &r)
+handle_mixrampdb(Client &client, Request args, gcc_unused Response &r)
 {
-	float db;
-	if (!args.Parse(0, db, r))
-		return CommandResult::ERROR;
-
+	float db = args.ParseFloat(0);
 	client.player_control.SetMixRampDb(db);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_mixrampdelay(Client &client, Request args, Response &r)
+handle_mixrampdelay(Client &client, Request args, gcc_unused Response &r)
 {
-	float delay_secs;
-	if (!args.Parse(0, delay_secs, r))
-		return CommandResult::ERROR;
-
+	float delay_secs = args.ParseFloat(0);
 	client.player_control.SetMixRampDelay(delay_secs);
-
 	return CommandResult::OK;
 }
 

src/command/PlaylistCommands.cxx

@@ -70,9 +70,7 @@ handle_save(Client &client, Request args, Response &r)
 CommandResult
 handle_load(Client &client, Request args, Response &r)
 {
-	RangeArg range = RangeArg::All();
-	if (!args.ParseOptional(1, range, r))
-		return CommandResult::ERROR;
+	RangeArg range = args.ParseOptional(1, RangeArg::All());
 
 	const ScopeBulkEdit bulk_edit(client.partition);
 
@@ -144,9 +142,7 @@ CommandResult
 handle_playlistdelete(gcc_unused Client &client, Request args, Response &r)
 {
 	const char *const name = args[0];
-	unsigned from;
-	if (!args.Parse(1, from, r))
-		return CommandResult::ERROR;
+	unsigned from = args.ParseUnsigned(1);
 
 	Error error;
 	return spl_remove_index(name, from, error)
@@ -158,9 +154,8 @@ CommandResult
 handle_playlistmove(gcc_unused Client &client, Request args, Response &r)
 {
 	const char *const name = args.front();
-	unsigned from, to;
-	if (!args.Parse(1, from, r) || !args.Parse(2, to, r))
-		return CommandResult::ERROR;
+	unsigned from = args.ParseUnsigned(1);
+	unsigned to = args.ParseUnsigned(2);
 
 	Error error;
 	return spl_move_index(name, from, to, error)

src/command/QueueCommands.cxx

@@ -128,9 +128,7 @@ handle_addid(Client &client, Request args, Response &r)
 		return print_error(r, error);
 
 	if (args.size == 2) {
-		unsigned to;
-		if (!args.Parse(1, to, r))
-			return CommandResult::ERROR;
+		unsigned to = args.ParseUnsigned(1);
 
 		try {
 			client.partition.MoveId(added_id, to);
@@ -180,9 +178,7 @@ parse_time_range(const char *p, SongTime &start_r, SongTime &end_r)
 CommandResult
 handle_rangeid(Client &client, Request args, Response &r)
 {
-	unsigned id;
-	if (!args.Parse(0, id, r))
-		return CommandResult::ERROR;
+	unsigned id = args.ParseUnsigned(0);
 
 	SongTime start, end;
 	if (!parse_time_range(args[1], start, end)) {
@@ -200,23 +196,17 @@ handle_rangeid(Client &client, Request args, Response &r)
 }
 
 CommandResult
-handle_delete(Client &client, Request args, Response &r)
+handle_delete(Client &client, Request args, gcc_unused Response &r)
 {
-	RangeArg range;
-	if (!args.Parse(0, range, r))
-		return CommandResult::ERROR;
-
+	RangeArg range = args.ParseRange(0);
 	client.partition.DeleteRange(range.start, range.end);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_deleteid(Client &client, Request args, Response &r)
+handle_deleteid(Client &client, Request args, gcc_unused Response &r)
 {
-	unsigned id;
-	if (!args.Parse(0, id, r))
-		return CommandResult::ERROR;
-
+	unsigned id = args.ParseUnsigned(0);
 	client.partition.DeleteId(id);
 	return CommandResult::OK;
 }
@@ -229,12 +219,9 @@ handle_playlist(Client &client, gcc_unused Request args, Response &r)
 }
 
 CommandResult
-handle_shuffle(gcc_unused Client &client, Request args, Response &r)
+handle_shuffle(gcc_unused Client &client, Request args, gcc_unused Response &r)
 {
-	RangeArg range = RangeArg::All();
-	if (!args.ParseOptional(0, range, r))
-		return CommandResult::ERROR;
-
+	RangeArg range = args.ParseOptional(0, RangeArg::All());
 	client.partition.Shuffle(range.start, range.end);
 	return CommandResult::OK;
 }
@@ -249,14 +236,8 @@ handle_clear(Client &client, gcc_unused Request args, gcc_unused Response &r)
 CommandResult
 handle_plchanges(Client &client, Request args, Response &r)
 {
-	uint32_t version;
-	if (!ParseCommandArg32(r, version, args.front()))
-		return CommandResult::ERROR;
-
-	RangeArg range = RangeArg::All();
-	if (!args.ParseOptional(1, range, r))
-		return CommandResult::ERROR;
-
+	uint32_t version = ParseCommandArgU32(args.front());
+	RangeArg range = args.ParseOptional(1, RangeArg::All());
 	playlist_print_changes_info(r, client.partition,
 				    client.playlist, version,
 				    range.start, range.end);
@@ -266,14 +247,8 @@ handle_plchanges(Client &client, Request args, Response &r)
 CommandResult
 handle_plchangesposid(Client &client, Request args, Response &r)
 {
-	uint32_t version;
-	if (!ParseCommandArg32(r, version, args.front()))
-		return CommandResult::ERROR;
-
-	RangeArg range = RangeArg::All();
-	if (!args.ParseOptional(1, range, r))
-		return CommandResult::ERROR;
-
+	uint32_t version = ParseCommandArgU32(args.front());
+	RangeArg range = args.ParseOptional(1, RangeArg::All());
 	playlist_print_changes_position(r, client.playlist, version,
 					range.start, range.end);
 	return CommandResult::OK;
@@ -282,9 +257,7 @@ handle_plchangesposid(Client &client, Request args, Response &r)
 CommandResult
 handle_playlistinfo(Client &client, Request args, Response &r)
 {
-	RangeArg range = RangeArg::All();
-	if (!args.ParseOptional(0, range, r))
-		return CommandResult::ERROR;
+	RangeArg range = args.ParseOptional(0, RangeArg::All());
 
 	if (!playlist_print_info(r, client.partition, client.playlist,
 				 range.start, range.end))
@@ -298,10 +271,7 @@ CommandResult
 handle_playlistid(Client &client, Request args, Response &r)
 {
 	if (!args.IsEmpty()) {
-		unsigned id;
-		if (!args.Parse(0, id, r))
-			return CommandResult::ERROR;
-
+		unsigned id = args.ParseUnsigned(0);
 		bool ret = playlist_print_id(r, client.partition,
 					     client.playlist, id);
 		if (!ret)
@@ -341,17 +311,13 @@ handle_playlistsearch(Client &client, Request args, Response &r)
 }
 
 CommandResult
-handle_prio(Client &client, Request args, Response &r)
+handle_prio(Client &client, Request args, gcc_unused Response &r)
 {
-	unsigned priority;
-	if (!args.ParseShift(0, priority, r, 0xff))
-		return CommandResult::ERROR;
+	unsigned priority = args.ParseUnsigned(0, 0xff);
+	args.shift();
 
 	for (const char *i : args) {
-		RangeArg range;
-		if (!ParseCommandArg(r, range, i))
-			return CommandResult::ERROR;
-
+		RangeArg range = ParseCommandArgRange(i);
 		client.partition.SetPriorityRange(range.start, range.end,
 						  priority);
 	}
@@ -360,17 +326,13 @@ handle_prio(Client &client, Request args, Response &r)
 }
 
 CommandResult
-handle_prioid(Client &client, Request args, Response &r)
+handle_prioid(Client &client, Request args, gcc_unused Response &r)
 {
-	unsigned priority;
-	if (!args.ParseShift(0, priority, r, 0xff))
-		return CommandResult::ERROR;
+	unsigned priority = args.ParseUnsigned(0, 0xff);
+	args.shift();
 
 	for (const char *i : args) {
-		unsigned song_id;
-		if (!ParseCommandArg(r, song_id, i))
-			return CommandResult::ERROR;
-
+		unsigned song_id = ParseCommandArgUnsigned(i);
 		client.partition.SetPriorityId(song_id, priority);
 	}
 
@@ -378,48 +340,37 @@ handle_prioid(Client &client, Request args, Response &r)
 }
 
 CommandResult
-handle_move(Client &client, Request args, Response &r)
+handle_move(Client &client, Request args, gcc_unused Response &r)
 {
-	RangeArg range;
-	int to;
-
-	if (!args.Parse(0, range, r) || !args.Parse(1, to, r))
-		return CommandResult::ERROR;
-
+	RangeArg range = args.ParseRange(0);
+	int to = args.ParseInt(1);
 	client.partition.MoveRange(range.start, range.end, to);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_moveid(Client &client, Request args, Response &r)
+handle_moveid(Client &client, Request args, gcc_unused Response &r)
 {
-	unsigned id;
-	int to;
-	if (!args.Parse(0, id, r) || !args.Parse(1, to, r))
-		return CommandResult::ERROR;
-
+	unsigned id = args.ParseUnsigned(0);
+	int to = args.ParseInt(1);
 	client.partition.MoveId(id, to);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_swap(Client &client, Request args, Response &r)
+handle_swap(Client &client, Request args, gcc_unused Response &r)
 {
-	unsigned song1, song2;
-	if (!args.Parse(0, song1, r) || !args.Parse(1, song2, r))
-		return CommandResult::ERROR;
-
+	unsigned song1 = args.ParseUnsigned(0);
+	unsigned song2 = args.ParseUnsigned(1);
 	client.partition.SwapPositions(song1, song2);
 	return CommandResult::OK;
 }
 
 CommandResult
-handle_swapid(Client &client, Request args, Response &r)
+handle_swapid(Client &client, Request args, gcc_unused Response &r)
 {
-	unsigned id1, id2;
-	if (!args.Parse(0, id1, r) || !args.Parse(1, id2, r))
-		return CommandResult::ERROR;
-
+	unsigned id1 = args.ParseUnsigned(0);
+	unsigned id2 = args.ParseUnsigned(1);
 	client.partition.SwapIds(id1, id2);
 	return CommandResult::OK;
 }

src/command/Request.hxx

@@ -22,6 +22,7 @@
 
 #include "check.h"
 #include "protocol/ArgParser.hxx"
+#include "Chrono.hxx"
 #include "util/ConstBuffer.hxx"
 
 #include <utility>
@@ -44,30 +45,72 @@ public:
 			     : default_value;
 	}
 
-	template<typename T, typename... Args>
-	bool Parse(unsigned idx, T &value_r, Response &r,
-		   Args&&... args) {
+	gcc_pure
+	int ParseInt(unsigned idx) const {
 		assert(idx < size);
+		return ParseCommandArgInt(data[idx]);
+	}
 
-		return ParseCommandArg(r, value_r, data[idx],
-				       std::forward<Args>(args)...);
+	gcc_pure
+	int ParseInt(unsigned idx, int min_value, int max_value) const {
+		assert(idx < size);
+		return ParseCommandArgInt(data[idx], min_value, max_value);
 	}
 
-	template<typename T, typename... Args>
-	bool ParseOptional(unsigned idx, T &value_r, Response &r,
-			   Args&&... args) {
-		return idx >= size ||
-			Parse(idx, value_r, r,
-			      std::forward<Args>(args)...);
+	gcc_pure
+	int ParseUnsigned(unsigned idx) const {
+		assert(idx < size);
+		return ParseCommandArgUnsigned(data[idx]);
 	}
 
-	template<typename T, typename... Args>
-	bool ParseShift(unsigned idx, T &value_r, Response &r,
-			Args&&... args) {
-		bool success = Parse(idx, value_r, r,
-				     std::forward<Args>(args)...);
-		shift();
-		return success;
+	gcc_pure
+	int ParseUnsigned(unsigned idx, unsigned max_value) const {
+		assert(idx < size);
+		return ParseCommandArgUnsigned(data[idx], max_value);
+	}
+
+	gcc_pure
+	bool ParseBool(unsigned idx) const {
+		assert(idx < size);
+		return ParseCommandArgBool(data[idx]);
+	}
+
+	gcc_pure
+	RangeArg ParseRange(unsigned idx) const {
+		assert(idx < size);
+		return ParseCommandArgRange(data[idx]);
+	}
+
+	gcc_pure
+	float ParseFloat(unsigned idx) const {
+		assert(idx < size);
+		return ParseCommandArgFloat(data[idx]);
+	}
+
+	gcc_pure
+	SongTime ParseSongTime(unsigned idx) const {
+		assert(idx < size);
+		return ParseCommandArgSongTime(data[idx]);
+	}
+
+	gcc_pure
+	SignedSongTime ParseSignedSongTime(unsigned idx) const {
+		assert(idx < size);
+		return ParseCommandArgSignedSongTime(data[idx]);
+	}
+
+	gcc_pure
+	int ParseOptional(unsigned idx, int default_value) const {
+		return idx < size
+			? ParseInt(idx)
+			: default_value;
+	}
+
+	gcc_pure
+	RangeArg ParseOptional(unsigned idx, RangeArg default_value) const {
+		return idx < size
+			? ParseRange(idx)
+			: default_value;
 	}
 };
 

src/command/TagCommands.cxx

@@ -30,9 +30,7 @@
 CommandResult
 handle_addtagid(Client &client, Request args, Response &r)
 {
-	unsigned song_id;
-	if (!args.Parse(0, song_id, r))
-		return CommandResult::ERROR;
+	unsigned song_id = args.ParseUnsigned(0);
 
 	const char *const tag_name = args[1];
 	const TagType tag_type = tag_name_parse_i(tag_name);
@@ -54,9 +52,7 @@ handle_addtagid(Client &client, Request args, Response &r)
 CommandResult
 handle_cleartagid(Client &client, Request args, Response &r)
 {
-	unsigned song_id;
-	if (!args.Parse(0, song_id, r))
-		return CommandResult::ERROR;
+	unsigned song_id = args.ParseUnsigned(0);
 
 	TagType tag_type = TAG_NUM_OF_ITEM_TYPES;
 	if (args.size >= 2) {

src/protocol/Ack.hxx

@@ -20,6 +20,10 @@
 #ifndef MPD_ACK_H
 #define MPD_ACK_H
 
+#include <stdexcept>
+
+#include <stdio.h>
+
 class Domain;
 
 enum ack {
@@ -40,4 +44,25 @@ enum ack {
 
 extern const Domain ack_domain;
 
+class ProtocolError : public std::runtime_error {
+	enum ack code;
+
+public:
+	ProtocolError(enum ack _code, const char *msg)
+		:std::runtime_error(msg), code(_code) {}
+
+	enum ack GetCode() const {
+		return code;
+	}
+};
+
+template<typename... Args>
+static inline ProtocolError
+FormatProtocolError(enum ack code, const char *fmt, Args&&... args) noexcept
+{
+	char buffer[256];
+	snprintf(buffer, sizeof(buffer), fmt, std::forward<Args>(args)...);
+	return ProtocolError(code, buffer);
+}
+
 #endif

src/protocol/ArgParser.cxx

@@ -19,199 +19,155 @@
 
 #include "config.h"
 #include "ArgParser.hxx"
+#include "Ack.hxx"
 #include "Chrono.hxx"
-#include "client/Response.hxx"
 
 #include <stdlib.h>
 
-bool
-ParseCommandArg32(Response &r, uint32_t &value_r, const char *s)
+uint32_t
+ParseCommandArgU32(const char *s)
 {
 	char *test;
+	auto value = strtoul(s, &test, 10);
+	if (test == s || *test != '\0')
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Integer expected: %s", s);
 
-	value_r = strtoul(s, &test, 10);
-	if (test == s || *test != '\0') {
-		r.FormatError(ACK_ERROR_ARG, "Integer expected: %s", s);
-		return false;
-	}
-	return true;
+	return value;
 }
 
-bool
-ParseCommandArg(Response &r, int &value_r, const char *s,
-		int min_value, int max_value)
+int
+ParseCommandArgInt(const char *s, int min_value, int max_value)
 {
 	char *test;
-	long value;
+	auto value = strtol(s, &test, 10);
+	if (test == s || *test != '\0')
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Integer expected: %s", s);
 
-	value = strtol(s, &test, 10);
-	if (test == s || *test != '\0') {
-		r.FormatError(ACK_ERROR_ARG, "Integer expected: %s", s);
-		return false;
-	}
+	if (value < min_value || value > max_value)
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Number too large: %s", s);
 
-	if (value < min_value || value > max_value) {
-		r.FormatError(ACK_ERROR_ARG, "Number too large: %s", s);
-		return false;
-	}
-
-	value_r = (int)value;
-	return true;
+	return (int)value;
 }
 
-bool
-ParseCommandArg(Response &r, int &value_r, const char *s)
+int
+ParseCommandArgInt(const char *s)
 {
-	return ParseCommandArg(r, value_r, s,
-			       std::numeric_limits<int>::min(),
-			       std::numeric_limits<int>::max());
+	return ParseCommandArgInt(s,
+				  std::numeric_limits<int>::min(),
+				  std::numeric_limits<int>::max());
 }
 
-bool
-ParseCommandArg(Response &r, RangeArg &value_r, const char *s)
+RangeArg
+ParseCommandArgRange(const char *s)
 {
 	char *test, *test2;
-	long value;
-
-	value = strtol(s, &test, 10);
-	if (test == s || (*test != '\0' && *test != ':')) {
-		r.FormatError(ACK_ERROR_ARG,
-			      "Integer or range expected: %s", s);
-		return false;
-	}
+	auto value = strtol(s, &test, 10);
+	if (test == s || (*test != '\0' && *test != ':'))
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Integer or range expected: %s", s);
 
-	if (value == -1 && *test == 0) {
+	if (value == -1 && *test == 0)
 		/* compatibility with older MPD versions: specifying
 		   "-1" makes MPD display the whole list */
-		value_r.start = 0;
-		value_r.end = std::numeric_limits<int>::max();
-		return true;
-	}
+		return RangeArg::All();
 
-	if (value < 0) {
-		r.FormatError(ACK_ERROR_ARG, "Number is negative: %s", s);
-		return false;
-	}
+	if (value < 0)
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Number is negative: %s", s);
 
-	if (unsigned(value) > std::numeric_limits<unsigned>::max()) {
-		r.FormatError(ACK_ERROR_ARG, "Number too large: %s", s);
-		return false;
-	}
+	if (unsigned(value) > std::numeric_limits<unsigned>::max())
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Number too large: %s", s);
 
-	value_r.start = (unsigned)value;
+	RangeArg range;
+	range.start = (unsigned)value;
 
 	if (*test == ':') {
 		value = strtol(++test, &test2, 10);
-		if (*test2 != '\0') {
-			r.FormatError(ACK_ERROR_ARG,
-				      "Integer or range expected: %s", s);
-			return false;
-		}
+		if (*test2 != '\0')
+			throw FormatProtocolError(ACK_ERROR_ARG,
+						  "Integer or range expected: %s",
+						  s);
 
 		if (test == test2)
 			value = std::numeric_limits<int>::max();
 
-		if (value < 0) {
-			r.FormatError(ACK_ERROR_ARG,
-				      "Number is negative: %s", s);
-			return false;
-		}
+		if (value < 0)
+			throw FormatProtocolError(ACK_ERROR_ARG,
+						  "Number is negative: %s", s);
 
-		if (unsigned(value) > std::numeric_limits<unsigned>::max()) {
-			r.FormatError(ACK_ERROR_ARG,
-				      "Number too large: %s", s);
-			return false;
-		}
+		if (unsigned(value) > std::numeric_limits<unsigned>::max())
+			throw FormatProtocolError(ACK_ERROR_ARG,
+						  "Number too large: %s", s);
 
-		value_r.end = (unsigned)value;
+		range.end = (unsigned)value;
 	} else {
-		value_r.end = (unsigned)value + 1;
+		range.end = (unsigned)value + 1;
 	}
 
-	return true;
+	return range;
 }
 
-bool
-ParseCommandArg(Response &r, unsigned &value_r, const char *s,
-		unsigned max_value)
+unsigned
+ParseCommandArgUnsigned(const char *s, unsigned max_value)
 {
-	unsigned long value;
 	char *endptr;
+	auto value = strtoul(s, &endptr, 10);
+	if (endptr == s || *endptr != 0)
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Integer expected: %s", s);
 
-	value = strtoul(s, &endptr, 10);
-	if (endptr == s || *endptr != 0) {
-		r.FormatError(ACK_ERROR_ARG, "Integer expected: %s", s);
-		return false;
-	}
-
-	if (value > max_value) {
-		r.FormatError(ACK_ERROR_ARG,
-			      "Number too large: %s", s);
-		return false;
-	}
+	if (value > max_value)
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Number too large: %s", s);
 
-	value_r = (unsigned)value;
-	return true;
+	return (unsigned)value;
 }
 
-bool
-ParseCommandArg(Response &r, unsigned &value_r, const char *s)
+unsigned
+ParseCommandArgUnsigned(const char *s)
 {
-	return ParseCommandArg(r, value_r, s,
-			       std::numeric_limits<unsigned>::max());
+	return ParseCommandArgUnsigned(s,
+				       std::numeric_limits<unsigned>::max());
 }
 
 bool
-ParseCommandArg(Response &r, bool &value_r, const char *s)
+ParseCommandArgBool(const char *s)
 {
-	long value;
 	char *endptr;
+	auto value = strtol(s, &endptr, 10);
+	if (endptr == s || *endptr != 0 || (value != 0 && value != 1))
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Boolean (0/1) expected: %s", s);
 
-	value = strtol(s, &endptr, 10);
-	if (endptr == s || *endptr != 0 || (value != 0 && value != 1)) {
-		r.FormatError(ACK_ERROR_ARG,
-			      "Boolean (0/1) expected: %s", s);
-		return false;
-	}
-
-	value_r = !!value;
-	return true;
+	return !!value;
 }
 
-bool
-ParseCommandArg(Response &r, float &value_r, const char *s)
+float
+ParseCommandArgFloat(const char *s)
 {
-	float value;
 	char *endptr;
+	auto value = strtof(s, &endptr);
+	if (endptr == s || *endptr != 0)
+		throw FormatProtocolError(ACK_ERROR_ARG,
+					  "Float expected: %s", s);
 
-	value = strtof(s, &endptr);
-	if (endptr == s || *endptr != 0) {
-		r.FormatError(ACK_ERROR_ARG, "Float expected: %s", s);
-		return false;
-	}
-
-	value_r = value;
-	return true;
+	return value;
 }
 
-bool
-ParseCommandArg(Response &r, SongTime &value_r, const char *s)
+SongTime
+ParseCommandArgSongTime(const char *s)
 {
-	float value;
-	bool success = ParseCommandArg(r, value, s) && value >= 0;
-	if (success)
-		value_r = SongTime::FromS(value);
-
-	return success;
+	auto value = ParseCommandArgFloat(s);
+	return SongTime::FromS(value);
 }
 
-bool
-ParseCommandArg(Response &r, SignedSongTime &value_r, const char *s)
+SignedSongTime
+ParseCommandArgSignedSongTime(const char *s)
 {
-	float value;
-	bool success = ParseCommandArg(r, value, s);
-	if (success)
-		value_r = SignedSongTime::FromS(value);
-
-	return success;
+	auto value = ParseCommandArgFloat(s);
+	return SongTime::FromS(value);
 }

src/protocol/ArgParser.hxx

@@ -21,6 +21,7 @@
 #define MPD_PROTOCOL_ARGPARSER_HXX
 
 #include "check.h"
+#include "Compiler.h"
 
 #include <limits>
 
@@ -30,15 +31,17 @@ class Response;
 class SongTime;
 class SignedSongTime;
 
-bool
-ParseCommandArg32(Response &r, uint32_t &value_r, const char *s);
+gcc_pure
+uint32_t
+ParseCommandArgU32(const char *s);
 
-bool
-ParseCommandArg(Response &r, int &value_r, const char *s,
-		int min_value, int max_value);
+gcc_pure
+int
+ParseCommandArgInt(const char *s, int min_value, int max_value);
 
-bool
-ParseCommandArg(Response &r, int &value_r, const char *s);
+gcc_pure
+int
+ParseCommandArgInt(const char *s);
 
 struct RangeArg {
 	unsigned start, end;
@@ -53,26 +56,32 @@ struct RangeArg {
 	}
 };
 
-bool
-ParseCommandArg(Response &r, RangeArg &value_r, const char *s);
+gcc_pure
+RangeArg
+ParseCommandArgRange(const char *s);
 
-bool
-ParseCommandArg(Response &r, unsigned &value_r, const char *s,
-		unsigned max_value);
+gcc_pure
+unsigned
+ParseCommandArgUnsigned(const char *s, unsigned max_value);
 
-bool
-ParseCommandArg(Response &r, unsigned &value_r, const char *s);
+gcc_pure
+unsigned
+ParseCommandArgUnsigned(const char *s);
 
+gcc_pure
 bool
-ParseCommandArg(Response &r, bool &value_r, const char *s);
+ParseCommandArgBool(const char *s);
 
-bool
-ParseCommandArg(Response &r, float &value_r, const char *s);
+gcc_pure
+float
+ParseCommandArgFloat(const char *s);
 
-bool
-ParseCommandArg(Response &r, SongTime &value_r, const char *s);
+gcc_pure
+SongTime
+ParseCommandArgSongTime(const char *s);
 
-bool
-ParseCommandArg(Response &r, SignedSongTime &value_r, const char *s);
+gcc_pure
+SignedSongTime
+ParseCommandArgSignedSongTime(const char *s);
 
 #endif

test/test_protocol.cxx

 #include "config.h"
 #include "protocol/ArgParser.hxx"
-#include "client/Response.hxx"
+#include "protocol/Ack.hxx"
 #include "Compiler.h"
 
 #include <cppunit/TestFixture.h>
@@ -10,20 +10,6 @@
 
 #include <stdlib.h>
 
-static enum ack last_error = ack(-1);
-
-void
-Response::Error(enum ack code, gcc_unused const char *msg)
-{
-	last_error = code;
-}
-
-void
-Response::FormatError(enum ack code, gcc_unused const char *fmt, ...)
-{
-	last_error = code;
-}
-
 class ArgParserTest : public CppUnit::TestFixture {
 	CPPUNIT_TEST_SUITE(ArgParserTest);
 	CPPUNIT_TEST(TestRange);
@@ -36,25 +22,24 @@ public:
 void
 ArgParserTest::TestRange()
 {
-	Client &client = *(Client *)nullptr;
-	Response r(client, 0);
-
-	RangeArg range;
-
-	CPPUNIT_ASSERT(ParseCommandArg(r, range, "1"));
+	RangeArg range = ParseCommandArgRange("1");
 	CPPUNIT_ASSERT_EQUAL(1u, range.start);
 	CPPUNIT_ASSERT_EQUAL(2u, range.end);
 
-	CPPUNIT_ASSERT(ParseCommandArg(r, range, "1:5"));
+	range = ParseCommandArgRange("1:5");
 	CPPUNIT_ASSERT_EQUAL(1u, range.start);
 	CPPUNIT_ASSERT_EQUAL(5u, range.end);
 
-	CPPUNIT_ASSERT(ParseCommandArg(r, range, "1:"));
+	range = ParseCommandArgRange("1:");
 	CPPUNIT_ASSERT_EQUAL(1u, range.start);
 	CPPUNIT_ASSERT(range.end >= 999999u);
 
-	CPPUNIT_ASSERT(!ParseCommandArg(r, range, "-2"));
-	CPPUNIT_ASSERT_EQUAL(ACK_ERROR_ARG, last_error);
+	try {
+		range = ParseCommandArgRange("-2");
+		CPPUNIT_ASSERT(false);
+	} catch (ProtocolError) {
+		CPPUNIT_ASSERT(true);
+	}
 }
 
 CPPUNIT_TEST_SUITE_REGISTRATION(ArgParserTest);