From e939d667d9cbe459add98790c5c8b92d6aa5fa36 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Fri, 18 Dec 2015 09:50:48 +0100
Subject: [PATCH] protocol/Ack: add exception class wrapping enum ack
---
src/command/CommandError.cxx | 2 +
src/command/DatabaseCommands.cxx | 3 +-
src/command/OtherCommands.cxx | 8 +-
src/command/OutputCommands.cxx | 12 +-
src/command/PlayerCommands.cxx | 89 ++++---------
src/command/PlaylistCommands.cxx | 13 +-
src/command/QueueCommands.cxx | 117 +++++-----------
src/command/Request.hxx | 81 ++++++++---
src/command/TagCommands.cxx | 8 +-
src/protocol/Ack.hxx | 25 ++++
src/protocol/ArgParser.cxx | 222 +++++++++++++------------------
src/protocol/ArgParser.hxx | 51 ++++---
test/test_protocol.cxx | 35 ++---
13 files changed, 289 insertions(+), 377 deletions(-)
diff --git a/src/command/CommandError.cxx b/src/command/CommandError.cxx
index 93fe2cc94..73d2060e7 100644
--- a/src/command/CommandError.cxx
+++ b/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 &) {
diff --git a/src/command/DatabaseCommands.cxx b/src/command/DatabaseCommands.cxx
index bfcf3aa54..046415e0f 100644
--- a/src/command/DatabaseCommands.cxx
+++ b/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();
diff --git a/src/command/OtherCommands.cxx b/src/command/OtherCommands.cxx
index a95dace91..0b298f33e 100644
--- a/src/command/OtherCommands.cxx
+++ b/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) {
diff --git a/src/command/OutputCommands.cxx b/src/command/OutputCommands.cxx
index 7bbe5f905..3a6234701 100644
--- a/src/command/OutputCommands.cxx
+++ b/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");
diff --git a/src/command/PlayerCommands.cxx b/src/command/PlayerCommands.cxx
index 54ef4f3e7..d058f6c7f 100644
--- a/src/command/PlayerCommands.cxx
+++ b/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;
}
diff --git a/src/command/PlaylistCommands.cxx b/src/command/PlaylistCommands.cxx
index 625e82055..2da64a80e 100644
--- a/src/command/PlaylistCommands.cxx
+++ b/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)
diff --git a/src/command/QueueCommands.cxx b/src/command/QueueCommands.cxx
index 75b3a6057..fad53f372 100644
--- a/src/command/QueueCommands.cxx
+++ b/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;
}
diff --git a/src/command/Request.hxx b/src/command/Request.hxx
index 1616b7045..cbc360370 100644
--- a/src/command/Request.hxx
+++ b/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 ParseCommandArg(r, value_r, data[idx],
- std::forward<Args>(args)...);
+ return ParseCommandArgInt(data[idx]);
}
- 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 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 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) const {
+ assert(idx < size);
+ return ParseCommandArgUnsigned(data[idx]);
+ }
+
+ 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;
}
};
diff --git a/src/command/TagCommands.cxx b/src/command/TagCommands.cxx
index 2a7076bdc..ac09ca6c1 100644
--- a/src/command/TagCommands.cxx
+++ b/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) {
diff --git a/src/protocol/Ack.hxx b/src/protocol/Ack.hxx
index c8457c5b4..d2d0c0aa0 100644
--- a/src/protocol/Ack.hxx
+++ b/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
diff --git a/src/protocol/ArgParser.cxx b/src/protocol/ArgParser.cxx
index 31756f53e..29266a387 100644
--- a/src/protocol/ArgParser.cxx
+++ b/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;
+ auto value = strtol(s, &test, 10);
+ if (test == s || (*test != '\0' && *test != ':'))
+ throw FormatProtocolError(ACK_ERROR_ARG,
+ "Integer or range expected: %s", s);
- value = strtol(s, &test, 10);
- if (test == s || (*test != '\0' && *test != ':')) {
- r.FormatError(ACK_ERROR_ARG,
- "Integer or range expected: %s", s);
- return false;
- }
-
- 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)
+ throw FormatProtocolError(ACK_ERROR_ARG,
+ "Number too large: %s", s);
- if (value > max_value) {
- r.FormatError(ACK_ERROR_ARG,
- "Number too large: %s", s);
- return false;
- }
+ return (unsigned)value;
+}
- value_r = (unsigned)value;
- return true;
+unsigned
+ParseCommandArgUnsigned(const char *s)
+{
+ return ParseCommandArgUnsigned(s,
+ std::numeric_limits<unsigned>::max());
}
bool
-ParseCommandArg(Response &r, unsigned &value_r, const char *s)
+ParseCommandArgBool(const char *s)
{
- return ParseCommandArg(r, value_r, s,
- std::numeric_limits<unsigned>::max());
-}
-
-bool
-ParseCommandArg(Response &r, bool &value_r, 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);
}
diff --git a/src/protocol/ArgParser.hxx b/src/protocol/ArgParser.hxx
index f60dbdf50..83c9934b0 100644
--- a/src/protocol/ArgParser.hxx
+++ b/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
diff --git a/test/test_protocol.cxx b/test/test_protocol.cxx
index e683fe2cb..d6c8447e5 100644
--- a/test/test_protocol.cxx
+++ b/test/test_protocol.cxx
@@ -1,6 +1,6 @@
#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);