diff options
author | Max Kellermann <max@duempel.org> | 2015-12-18 09:50:48 +0100 |
---|---|---|
committer | Max Kellermann <max@duempel.org> | 2015-12-18 09:53:02 +0100 |
commit | e939d667d9cbe459add98790c5c8b92d6aa5fa36 (patch) | |
tree | abb4fda3a28dd153439ccb544fabed33a2c438f1 | |
parent | 8bb5a565cd9060b3b98bb7b9853fe99703fc18da (diff) |
protocol/Ack: add exception class wrapping enum ack
-rw-r--r-- | src/command/CommandError.cxx | 2 | ||||
-rw-r--r-- | src/command/DatabaseCommands.cxx | 3 | ||||
-rw-r--r-- | src/command/OtherCommands.cxx | 8 | ||||
-rw-r--r-- | src/command/OutputCommands.cxx | 12 | ||||
-rw-r--r-- | src/command/PlayerCommands.cxx | 89 | ||||
-rw-r--r-- | src/command/PlaylistCommands.cxx | 13 | ||||
-rw-r--r-- | src/command/QueueCommands.cxx | 117 | ||||
-rw-r--r-- | src/command/Request.hxx | 79 | ||||
-rw-r--r-- | src/command/TagCommands.cxx | 8 | ||||
-rw-r--r-- | src/protocol/Ack.hxx | 25 | ||||
-rw-r--r-- | src/protocol/ArgParser.cxx | 218 | ||||
-rw-r--r-- | src/protocol/ArgParser.hxx | 51 | ||||
-rw-r--r-- | test/test_protocol.cxx | 35 |
13 files changed, 286 insertions, 374 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 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; } }; 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; - - 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); } 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); |