From 49009573bc2eacd823d57433daf1f59dfe415065 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 28 Mar 2014 16:59:26 +0100 Subject: [PATCH] Don't interpret strings as format strings Ludo reported this error: unexpected Nix daemon error: boost::too_few_args: format-string refered to more arguments than were passed coming from this line: printMsg(lvlError, run.program + ": " + string(err, 0, p)); The problem here is that the string ends up implicitly converted to a Boost format() object, so % characters are treated specially. I always assumed (wrongly) that strings are converted to a format object that outputs the string as-is. Since this assumption appears in several places that may be hard to grep for, I've added some C++ type hackery to ensures that the right thing happens. So you don't have to worry about % in statements like printMsg(lvlError, "foo: " + s); or throw Error("foo: " + s); --- src/libutil/types.hh | 23 ++++++++++++++++------- src/libutil/util.cc | 26 +++++++++++++------------- src/libutil/util.hh | 6 +++--- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/libutil/types.hh b/src/libutil/types.hh index a2ab1cc0e..4b5ce9a78 100644 --- a/src/libutil/types.hh +++ b/src/libutil/types.hh @@ -20,28 +20,37 @@ using std::vector; using boost::format; +struct FormatOrString +{ + string s; + FormatOrString(const string & s) : s(s) { }; + FormatOrString(const format & f) : s(f.str()) { }; + FormatOrString(const char * s) : s(s) { }; +}; + + /* BaseError should generally not be caught, as it has Interrupted as a subclass. Catch Error instead. */ -class BaseError : public std::exception +class BaseError : public std::exception { protected: string prefix_; // used for location traces etc. string err; public: unsigned int status; // exit status - BaseError(const format & f, unsigned int status = 1); + BaseError(const FormatOrString & fs, unsigned int status = 1); ~BaseError() throw () { }; const char * what() const throw () { return err.c_str(); } const string & msg() const throw () { return err; } const string & prefix() const throw () { return prefix_; } - BaseError & addPrefix(const format & f); + BaseError & addPrefix(const FormatOrString & fs); }; #define MakeError(newClass, superClass) \ class newClass : public superClass \ { \ public: \ - newClass(const format & f, unsigned int status = 1) : superClass(f, status) { }; \ + newClass(const FormatOrString & fs, unsigned int status = 1) : superClass(fs, status) { }; \ }; MakeError(Error, BaseError) @@ -50,7 +59,7 @@ class SysError : public Error { public: int errNo; - SysError(const format & f); + SysError(const FormatOrString & fs); }; @@ -63,8 +72,8 @@ typedef string Path; typedef list Paths; typedef set PathSet; - -typedef enum { + +typedef enum { lvlError = 0, lvlInfo, lvlTalkative, diff --git a/src/libutil/util.cc b/src/libutil/util.cc index b264fc5f3..15c462ce4 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -25,22 +25,22 @@ extern char * * environ; namespace nix { -BaseError::BaseError(const format & f, unsigned int status) +BaseError::BaseError(const FormatOrString & fs, unsigned int status) : status(status) { - err = f.str(); + err = fs.s; } -BaseError & BaseError::addPrefix(const format & f) +BaseError & BaseError::addPrefix(const FormatOrString & fs) { - prefix_ = f.str() + prefix_; + prefix_ = fs.s + prefix_; return *this; } -SysError::SysError(const format & f) - : Error(format("%1%: %2%") % f.str() % strerror(errno)) +SysError::SysError(const FormatOrString & fs) + : Error(format("%1%: %2%") % fs.s % strerror(errno)) , errNo(errno) { } @@ -417,14 +417,14 @@ static string escVerbosity(Verbosity level) } -void Nest::open(Verbosity level, const format & f) +void Nest::open(Verbosity level, const FormatOrString & fs) { if (level <= verbosity) { if (logType == ltEscapes) std::cerr << "\033[" << escVerbosity(level) << "p" - << f.str() << "\n"; + << fs.s << "\n"; else - printMsg_(level, f); + printMsg_(level, fs); nest = true; nestingLevel++; } @@ -442,7 +442,7 @@ void Nest::close() } -void printMsg_(Verbosity level, const format & f) +void printMsg_(Verbosity level, const FormatOrString & fs) { checkInterrupt(); if (level > verbosity) return; @@ -452,15 +452,15 @@ void printMsg_(Verbosity level, const format & f) prefix += "| "; else if (logType == ltEscapes && level != lvlInfo) prefix = "\033[" + escVerbosity(level) + "s"; - string s = (format("%1%%2%\n") % prefix % f.str()).str(); + string s = (format("%1%%2%\n") % prefix % fs.s).str(); writeToStderr(s); } -void warnOnce(bool & haveWarned, const format & f) +void warnOnce(bool & haveWarned, const FormatOrString & fs) { if (!haveWarned) { - printMsg(lvlError, format("warning: %1%") % f.str()); + printMsg(lvlError, format("warning: %1%") % fs.s); haveWarned = true; } } diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 5d0408f9b..8bedfea9a 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -125,11 +125,11 @@ private: public: Nest(); ~Nest(); - void open(Verbosity level, const format & f); + void open(Verbosity level, const FormatOrString & fs); void close(); }; -void printMsg_(Verbosity level, const format & f); +void printMsg_(Verbosity level, const FormatOrString & fs); #define startNest(varName, level, f) \ Nest varName; \ @@ -146,7 +146,7 @@ void printMsg_(Verbosity level, const format & f); #define debug(f) printMsg(lvlDebug, f) -void warnOnce(bool & haveWarned, const format & f); +void warnOnce(bool & haveWarned, const FormatOrString & fs); void writeToStderr(const string & s);