properly handle boost::bad_lexical_cast exceptions

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 \

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<int>(crossfade);
+	int cf = fromString<unsigned>(crossfade);
-	if (cf > 0)
+	lowerBoundCheck(cf, 1);
-	{
+	Config.crossfade_time = cf;
-		Config.crossfade_time = cf;
+	Mpd.SetCrossfade(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<int>(strvolume);
+	int volume = fromString<unsigned>(strvolume);
-	if (volume >= 0 && volume <= 100)
+	boundsCheck(volume, 0, 100);
-	{
+	Mpd.SetVolume(volume);
-		Mpd.SetVolume(volume);
+	Statusbar::msg("Volume set to %d%%", 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<std::string>(Config.locked_screen_width_part*100));
+			std::string strpart = wFooter->getString(boost::lexical_cast<std::string>(part));
 			Statusbar::unlock();
-			if (str_part.empty())
+			part = fromString<unsigned>(strpart);
-				return;
-			part = boost::lexical_cast<int>(str_part);
-		}
-		if (part < 20 || part > 80)
-		{
-			Statusbar::msg("Number is out of range");
-			return;
 		}
+		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)): ";
+	Statusbar::put() << "Position to go (in %/m:ss/seconds(s)): ";
-	std::string position = wFooter->getString();
+	std::string strpos = wFooter->getString();
 	Statusbar::unlock();
 	
-	if (position.empty())
+	boost::regex rx;
-		return;
+	boost::smatch what;
 	
-	unsigned newpos = 0;
+	if (boost::regex_match(strpos, what, rx.assign("([0-9]+):([0-9]{2})"))) // mm:ss
-	size_t special_pos;
-	if ((special_pos = position.find(':')) != std::string::npos) // probably time in mm:ss
 	{
-		newpos = boost::lexical_cast<int>(position.substr(0, special_pos))*60
+		int mins = fromString<int>(what[1]);
-		       + boost::lexical_cast<int>(position.substr(special_pos +1));
+		int secs = fromString<int>(what[2]);
-		if (newpos <= myPlaylist->currentSongLength())
+		boundsCheck(secs, 0, 60);
-			Mpd.Seek(s.getPosition(), newpos);
+		Mpd.Seek(s.getPosition(), mins * 60 + secs);
-		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());
 	}
-	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);
+		int secs = fromString<int>(what[1]);
-		newpos = boost::lexical_cast<int>(position);
+		Mpd.Seek(s.getPosition(), secs);
-		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);
 	}
-	else
+	else if (boost::regex_match(strpos, what, rx.assign("([0-9]+)[%]{0,1}"))) // position in %
 	{
-		special_pos = position.find('%');
+		int percent = fromString<int>(what[1]);
-		if (special_pos != std::string::npos)
+		boundsCheck(percent, 0, 100);
-			position.resize(special_pos);
+		int secs = (percent * s.getDuration()) / 100.0;
-		newpos = boost::lexical_cast<int>(position);
+		Mpd.Seek(s.getPosition(), secs);
-		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);
 	}
+	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<size_t>(wFooter->getString());
+	std::string strnum = wFooter->getString();
 	Statusbar::unlock();
+	size_t number = fromString<size_t>(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))
+	unsigned prio = fromString<unsigned>(strprio);
-		return;
+	boundsCheck(prio, 0u, 255u);
-	int prio = boost::lexical_cast<int>(strprio);
-	if (prio < 0 || prio > 255)
-	{
-		Statusbar::msg("Number is out of range");
-		return;
-	}
 	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))
+	unsigned prio = fromString<unsigned>(strprio);
-		return;
+	boundsCheck(prio, 0u, 255u);
-	unsigned prio = boost::lexical_cast<unsigned>(strprio);
 	myPlaylist->main().filter(myPlaylist->main().begin(), myPlaylist->main().end(),
 		[prio](const NC::Menu<MPD::Song>::Item &s) {
 			return s.value().getPrio() > prio;

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);
+			try
-			for (; k.first != k.second; ++k.first)
+			{
-				if (k.first->execute())
+				auto k = Bindings.get(input);
-					break;
+				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();

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<TagLib::uint>(s.getDate()));
+	try {
-	tag->setTrack(boost::lexical_cast<TagLib::uint>(s.getTrack()));
+		tag->setYear(boost::lexical_cast<TagLib::uint>(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<TagLib::uint>(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()));
 }

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
 

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 <boost/format.hpp>
+#include <boost/lexical_cast.hpp>
+#include <boost/type_traits/is_unsigned.hpp>
+
+#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 <typename Type>
+	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 <typename Type>
+	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 <typename Type>
+	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 <typename TargetT, bool isUnsigned>
+struct unsigned_checker
+{
+	static void apply(const std::string &) { }
+};
+template <typename TargetT>
+struct unsigned_checker<TargetT, true>
+{
+	static void apply(const std::string &s)
+	{
+		if (s[0] == '-')
+			throw ConversionError(s);
+	}
+};
+
+template <typename TargetT>
+TargetT fromString(const std::string &source)
+{
+	unsigned_checker<TargetT, boost::is_unsigned<TargetT>::value>::apply(source);
+	try
+	{
+		return boost::lexical_cast<TargetT>(source);
+	}
+	catch (boost::bad_lexical_cast &)
+	{
+		throw ConversionError(source);
+	}
+}
+
+template <typename Type>
+void boundsCheck(const Type &value, const Type &lbound, const Type &ubound)
+{
+	if (value < lbound || value > ubound)
+		OutOfBounds::raise(value, lbound, ubound);
+}
+
+template <typename Type>
+void lowerBoundCheck(const Type &value, const Type &lbound)
+{
+	if (value < lbound)
+		OutOfBounds::raiseLower(value, lbound);
+}
+
+template <typename Type>
+void upperBoundCheck(const Type &value, const Type &ubound)
+{
+	if (value > ubound)
+		OutOfBounds::raiseUpper(value, ubound);
+}

src/utility/string.cpp

@@ -23,17 +23,6 @@
 #include <algorithm>
 #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("/");

src/utility/string.h

@@ -31,8 +31,6 @@ template <size_t N> 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);