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);
This commit is contained in:
Eelco Dolstra 2014-03-28 16:59:26 +01:00
parent 24cb65efc3
commit 49009573bc
3 changed files with 32 additions and 23 deletions

View file

@ -20,28 +20,37 @@ using std::vector;
using boost::format; 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 /* BaseError should generally not be caught, as it has Interrupted as
a subclass. Catch Error instead. */ a subclass. Catch Error instead. */
class BaseError : public std::exception class BaseError : public std::exception
{ {
protected: protected:
string prefix_; // used for location traces etc. string prefix_; // used for location traces etc.
string err; string err;
public: public:
unsigned int status; // exit status unsigned int status; // exit status
BaseError(const format & f, unsigned int status = 1); BaseError(const FormatOrString & fs, unsigned int status = 1);
~BaseError() throw () { }; ~BaseError() throw () { };
const char * what() const throw () { return err.c_str(); } const char * what() const throw () { return err.c_str(); }
const string & msg() const throw () { return err; } const string & msg() const throw () { return err; }
const string & prefix() const throw () { return prefix_; } const string & prefix() const throw () { return prefix_; }
BaseError & addPrefix(const format & f); BaseError & addPrefix(const FormatOrString & fs);
}; };
#define MakeError(newClass, superClass) \ #define MakeError(newClass, superClass) \
class newClass : public superClass \ class newClass : public superClass \
{ \ { \
public: \ 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) MakeError(Error, BaseError)
@ -50,7 +59,7 @@ class SysError : public Error
{ {
public: public:
int errNo; int errNo;
SysError(const format & f); SysError(const FormatOrString & fs);
}; };
@ -63,8 +72,8 @@ typedef string Path;
typedef list<Path> Paths; typedef list<Path> Paths;
typedef set<Path> PathSet; typedef set<Path> PathSet;
typedef enum { typedef enum {
lvlError = 0, lvlError = 0,
lvlInfo, lvlInfo,
lvlTalkative, lvlTalkative,

View file

@ -25,22 +25,22 @@ extern char * * environ;
namespace nix { namespace nix {
BaseError::BaseError(const format & f, unsigned int status) BaseError::BaseError(const FormatOrString & fs, unsigned int status)
: status(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; return *this;
} }
SysError::SysError(const format & f) SysError::SysError(const FormatOrString & fs)
: Error(format("%1%: %2%") % f.str() % strerror(errno)) : Error(format("%1%: %2%") % fs.s % strerror(errno))
, errNo(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 (level <= verbosity) {
if (logType == ltEscapes) if (logType == ltEscapes)
std::cerr << "\033[" << escVerbosity(level) << "p" std::cerr << "\033[" << escVerbosity(level) << "p"
<< f.str() << "\n"; << fs.s << "\n";
else else
printMsg_(level, f); printMsg_(level, fs);
nest = true; nest = true;
nestingLevel++; nestingLevel++;
} }
@ -442,7 +442,7 @@ void Nest::close()
} }
void printMsg_(Verbosity level, const format & f) void printMsg_(Verbosity level, const FormatOrString & fs)
{ {
checkInterrupt(); checkInterrupt();
if (level > verbosity) return; if (level > verbosity) return;
@ -452,15 +452,15 @@ void printMsg_(Verbosity level, const format & f)
prefix += "| "; prefix += "| ";
else if (logType == ltEscapes && level != lvlInfo) else if (logType == ltEscapes && level != lvlInfo)
prefix = "\033[" + escVerbosity(level) + "s"; 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); writeToStderr(s);
} }
void warnOnce(bool & haveWarned, const format & f) void warnOnce(bool & haveWarned, const FormatOrString & fs)
{ {
if (!haveWarned) { if (!haveWarned) {
printMsg(lvlError, format("warning: %1%") % f.str()); printMsg(lvlError, format("warning: %1%") % fs.s);
haveWarned = true; haveWarned = true;
} }
} }

View file

@ -125,11 +125,11 @@ private:
public: public:
Nest(); Nest();
~Nest(); ~Nest();
void open(Verbosity level, const format & f); void open(Verbosity level, const FormatOrString & fs);
void close(); void close();
}; };
void printMsg_(Verbosity level, const format & f); void printMsg_(Verbosity level, const FormatOrString & fs);
#define startNest(varName, level, f) \ #define startNest(varName, level, f) \
Nest varName; \ Nest varName; \
@ -146,7 +146,7 @@ void printMsg_(Verbosity level, const format & f);
#define debug(f) printMsg(lvlDebug, 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); void writeToStderr(const string & s);