From 88a3bdb5074e5f6d45129fe9f738df2c6080d0ff Mon Sep 17 00:00:00 2001 From: Andrzej Rybczak Date: Sat, 14 Sep 2013 17:26:45 +0200 Subject: [PATCH] properly handle boost::bad_lexical_cast exceptions --- src/Makefile.am | 1 + src/actions.cpp | 99 +++++++++++++-------------------- src/ncmpcpp.cpp | 20 +++++-- src/tags.cpp | 12 +++- src/title.cpp | 2 +- src/utility/conversion.h | 117 +++++++++++++++++++++++++++++++++++++++ src/utility/string.cpp | 11 ---- src/utility/string.h | 2 - 8 files changed, 182 insertions(+), 82 deletions(-) create mode 100644 src/utility/conversion.h diff --git a/src/Makefile.am b/src/Makefile.am index e1fb1bcb..c807aa64 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -55,6 +55,7 @@ INCLUDES= $(all_includes) ncmpcpp_LDFLAGS = $(all_libraries) noinst_HEADERS = \ utility/comparators.h \ + utility/conversion.h \ utility/html.h \ utility/string.h \ utility/type_conversions.h \ diff --git a/src/actions.cpp b/src/actions.cpp index 5f6811c9..539db316 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -36,6 +36,7 @@ #include "helpers.h" #include "statusbar.h" #include "utility/comparators.h" +#include "utility/conversion.h" #include "bindings.h" #include "browser.h" @@ -1194,12 +1195,10 @@ void SetCrossfade::run() Statusbar::put() << "Set crossfade to: "; std::string crossfade = wFooter->getString(3); Statusbar::unlock(); - int cf = boost::lexical_cast(crossfade); - if (cf > 0) - { - Config.crossfade_time = cf; - Mpd.SetCrossfade(cf); - } + int cf = fromString(crossfade); + lowerBoundCheck(cf, 1); + Config.crossfade_time = cf; + Mpd.SetCrossfade(cf); } void SetVolume::run() @@ -1210,12 +1209,10 @@ void SetVolume::run() Statusbar::put() << "Set volume to: "; std::string strvolume = wFooter->getString(3); Statusbar::unlock(); - int volume = boost::lexical_cast(strvolume); - if (volume >= 0 && volume <= 100) - { - Mpd.SetVolume(volume); - Statusbar::msg("Volume set to %d%%", volume); - } + int volume = fromString(strvolume); + boundsCheck(volume, 0, 100); + Mpd.SetVolume(volume); + Statusbar::msg("Volume set to %d%%", volume); } bool EditSong::canBeRun() const @@ -1519,17 +1516,11 @@ void ToggleScreenLock::run() { Statusbar::lock(); Statusbar::put() << "% of the locked screen's width to be reserved (20-80): "; - std::string str_part = wFooter->getString(boost::lexical_cast(Config.locked_screen_width_part*100)); + std::string strpart = wFooter->getString(boost::lexical_cast(part)); Statusbar::unlock(); - if (str_part.empty()) - return; - part = boost::lexical_cast(str_part); - } - if (part < 20 || part > 80) - { - Statusbar::msg("Number is out of range"); - return; + part = fromString(strpart); } + boundsCheck(part, 20, 80); Config.locked_screen_width_part = part/100.0; if (myScreen->lock()) Statusbar::msg("Screen locked (with %d%% width)", part); @@ -1568,44 +1559,34 @@ void JumpToPositionInSong::run() const MPD::Song s = myPlaylist->nowPlayingSong(); Statusbar::lock(); - Statusbar::put() << "Position to go (in %/mm:ss/seconds(s)): "; - std::string position = wFooter->getString(); + Statusbar::put() << "Position to go (in %/m:ss/seconds(s)): "; + std::string strpos = wFooter->getString(); Statusbar::unlock(); - if (position.empty()) - return; + boost::regex rx; + boost::smatch what; - unsigned newpos = 0; - size_t special_pos; - if ((special_pos = position.find(':')) != std::string::npos) // probably time in mm:ss + if (boost::regex_match(strpos, what, rx.assign("([0-9]+):([0-9]{2})"))) // mm:ss { - newpos = boost::lexical_cast(position.substr(0, special_pos))*60 - + boost::lexical_cast(position.substr(special_pos +1)); - if (newpos <= myPlaylist->currentSongLength()) - Mpd.Seek(s.getPosition(), newpos); - else - Statusbar::msg("Out of bounds, 0:00-%s possible for mm:ss, %s given", s.getLength().c_str(), MPD::Song::ShowTime(newpos).c_str()); + int mins = fromString(what[1]); + int secs = fromString(what[2]); + boundsCheck(secs, 0, 60); + Mpd.Seek(s.getPosition(), mins * 60 + secs); } - else if ((special_pos = position.find('s')) != std::string::npos) // probably position in seconds + else if (boost::regex_match(strpos, what, rx.assign("([0-9]+)s"))) // position in seconds { - position.resize(special_pos); - newpos = boost::lexical_cast(position); - if (newpos <= s.getDuration()) - Mpd.Seek(s.getPosition(), newpos); - else - Statusbar::msg("Out of bounds, 0-%d possible for seconds, %d given", s.getDuration(), newpos); + int secs = fromString(what[1]); + Mpd.Seek(s.getPosition(), secs); } - else + else if (boost::regex_match(strpos, what, rx.assign("([0-9]+)[%]{0,1}"))) // position in % { - special_pos = position.find('%'); - if (special_pos != std::string::npos) - position.resize(special_pos); - newpos = boost::lexical_cast(position); - if (newpos <= 100) - Mpd.Seek(s.getPosition(), s.getDuration()*newpos/100.0); - else - Statusbar::msg("Out of bounds, 0-100 possible for %%, %d given", newpos); + int percent = fromString(what[1]); + boundsCheck(percent, 0, 100); + int secs = (percent * s.getDuration()) / 100.0; + Mpd.Seek(s.getPosition(), secs); } + else + Statusbar::msg("Invalid format ([m]:[ss], [s]s, [%%]%%, [%%] accepted)"); } bool ReverseSelection::canBeRun() const @@ -1968,8 +1949,9 @@ void AddRandomItems::run() Statusbar::lock(); Statusbar::put() << "Number of random " << tag_type_str << "s: "; - size_t number = boost::lexical_cast(wFooter->getString()); + std::string strnum = wFooter->getString(); Statusbar::unlock(); + size_t number = fromString(strnum); if (number && (answer == 's' ? Mpd.AddRandomSongs(number) : Mpd.AddRandomTag(tag_type, number))) Statusbar::msg("%zu random %s%s added to playlist", number, tag_type_str.c_str(), number == 1 ? "" : "s"); } @@ -2093,14 +2075,8 @@ void SetSelectedItemsPriority::run() Statusbar::put() << "Set priority [0-255]: "; std::string strprio = wFooter->getString(); Statusbar::unlock(); - if (!isInteger(strprio.c_str(), true)) - return; - int prio = boost::lexical_cast(strprio); - if (prio < 0 || prio > 255) - { - Statusbar::msg("Number is out of range"); - return; - } + unsigned prio = fromString(strprio); + boundsCheck(prio, 0u, 255u); myPlaylist->SetSelectedItemsPriority(prio); } @@ -2117,9 +2093,8 @@ void FilterPlaylistOnPriorities::run() Statusbar::put() << "Show songs with priority higher than: "; std::string strprio = wFooter->getString(); Statusbar::unlock(); - if (!isInteger(strprio.c_str(), false)) - return; - unsigned prio = boost::lexical_cast(strprio); + unsigned prio = fromString(strprio); + boundsCheck(prio, 0u, 255u); myPlaylist->main().filter(myPlaylist->main().begin(), myPlaylist->main().end(), [prio](const NC::Menu::Item &s) { return s.value().getPrio() > prio; diff --git a/src/ncmpcpp.cpp b/src/ncmpcpp.cpp index 96c95580..21f58c46 100644 --- a/src/ncmpcpp.cpp +++ b/src/ncmpcpp.cpp @@ -47,6 +47,7 @@ #include "statusbar.h" #include "visualizer.h" #include "title.h" +#include "utility/conversion.h" namespace boost {// @@ -251,10 +252,21 @@ int main(int argc, char **argv) if (input == Key::noOp) continue; - auto k = Bindings.get(input); - for (; k.first != k.second; ++k.first) - if (k.first->execute()) - break; + try + { + auto k = Bindings.get(input); + for (; k.first != k.second; ++k.first) + if (k.first->execute()) + break; + } + catch (ConversionError &e) + { + Statusbar::msg("Couldn't convert value '%s' to target type", e.value().c_str()); + } + catch (OutOfBounds &e) + { + Statusbar::msg("%s", e.errorMessage().c_str()); + } if (myScreen == myPlaylist) myPlaylist->EnableHighlighting(); diff --git a/src/tags.cpp b/src/tags.cpp index 8042ca95..a8fb0cac 100644 --- a/src/tags.cpp +++ b/src/tags.cpp @@ -131,8 +131,16 @@ void writeCommonTags(const MPD::MutableSong &s, TagLib::Tag *tag) tag->setTitle(ToWString(s.getTitle())); tag->setArtist(ToWString(s.getArtist())); tag->setAlbum(ToWString(s.getAlbum())); - tag->setYear(boost::lexical_cast(s.getDate())); - tag->setTrack(boost::lexical_cast(s.getTrack())); + try { + tag->setYear(boost::lexical_cast(s.getDate())); + } catch (boost::bad_lexical_cast &) { + std::cerr << "writeCommonTags: couldn't write 'year' tag to '" << s.getURI() << "' as it's not a positive integer\n"; + } + try { + tag->setTrack(boost::lexical_cast(s.getTrack())); + } catch (boost::bad_lexical_cast &) { + std::cerr << "writeCommonTags: couldn't write 'track' tag to '" << s.getURI() << "' as it's not a positive integer\n"; + } tag->setGenre(ToWString(s.getGenre())); tag->setComment(ToWString(s.getComment())); } diff --git a/src/title.cpp b/src/title.cpp index 407c07b0..783961ac 100644 --- a/src/title.cpp +++ b/src/title.cpp @@ -32,7 +32,7 @@ void windowTitle(const std::string &) { } void windowTitle(const std::string &status) { if (strcmp(getenv("TERM"), "linux") && Config.set_window_title) - std::cout << "\033]0;" << status << "\7"; + std::cout << "\033]0;" << status << "\7" << std::flush; } #endif // USE_PDCURSES diff --git a/src/utility/conversion.h b/src/utility/conversion.h new file mode 100644 index 00000000..ca9f929d --- /dev/null +++ b/src/utility/conversion.h @@ -0,0 +1,117 @@ +/*************************************************************************** + * Copyright (C) 2008-2013 by Andrzej Rybczak * + * electricityispower@gmail.com * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program is distributed in the hope that it will be useful, * + * but WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * + * GNU General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA. * + ***************************************************************************/ + +#include +#include +#include + +#include "gcc.h" + +struct ConversionError +{ + ConversionError(std::string source) : m_source_value(source) { } + + const std::string &value() { return m_source_value; } + +private: + std::string m_target_type; + std::string m_source_value; +}; + +struct OutOfBounds +{ + const std::string &errorMessage() { return m_error_message; } + + template + GNUC_NORETURN static void raise(const Type &value, const Type &lbound, const Type &ubound) + { + throw OutOfBounds((boost::format( + "Value is out of bounds ([%1%, %2%] expected, %3% given)") % lbound % ubound % value).str()); + } + + template + GNUC_NORETURN static void raiseLower(const Type &value, const Type &lbound) + { + throw OutOfBounds((boost::format( + "Value is out of bounds ([%1%, ->) expected, %2% given)") % lbound % value).str()); + } + + template + GNUC_NORETURN static void raiseUpper(const Type &value, const Type &ubound) + { + throw OutOfBounds((boost::format( + "Value is out of bounds ((<-, %1%] expected, %2% given)") % ubound % value).str()); + } + +private: + OutOfBounds(std::string msg) : m_error_message(msg) { } + + std::string m_error_message; +}; + +template +struct unsigned_checker +{ + static void apply(const std::string &) { } +}; +template +struct unsigned_checker +{ + static void apply(const std::string &s) + { + if (s[0] == '-') + throw ConversionError(s); + } +}; + +template +TargetT fromString(const std::string &source) +{ + unsigned_checker::value>::apply(source); + try + { + return boost::lexical_cast(source); + } + catch (boost::bad_lexical_cast &) + { + throw ConversionError(source); + } +} + +template +void boundsCheck(const Type &value, const Type &lbound, const Type &ubound) +{ + if (value < lbound || value > ubound) + OutOfBounds::raise(value, lbound, ubound); +} + +template +void lowerBoundCheck(const Type &value, const Type &lbound) +{ + if (value < lbound) + OutOfBounds::raiseLower(value, lbound); +} + +template +void upperBoundCheck(const Type &value, const Type &ubound) +{ + if (value > ubound) + OutOfBounds::raiseUpper(value, ubound); +} diff --git a/src/utility/string.cpp b/src/utility/string.cpp index 3b74ca44..55071d20 100644 --- a/src/utility/string.cpp +++ b/src/utility/string.cpp @@ -23,17 +23,6 @@ #include #include "utility/string.h" -bool isInteger(const char *s, bool accept_signed) -{ - assert(s); - if (*s == '\0') - return false; - for (const char *it = s; *it != '\0'; ++it) - if (!(isdigit(*it) || (accept_signed && it == s && *it == '-'))) - return false; - return true; -} - std::string getBasename(const std::string &path) { size_t slash = path.rfind("/"); diff --git a/src/utility/string.h b/src/utility/string.h index 268cd017..247dab17 100644 --- a/src/utility/string.h +++ b/src/utility/string.h @@ -31,8 +31,6 @@ template size_t const_strlen(const char (&)[N]) { return N-1; } -bool isInteger(const char *s, bool accept_signed); - std::string getBasename(const std::string &path); std::string getParentDirectory(const std::string &path); std::string getSharedDirectory(const std::string &dir1, const std::string &dir2);