From 2794d881c15efef0de4b51cc1a8f0649181d7af9 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 30 Dec 2016 20:04:34 +0000 Subject: Use compile time formatter in place of boost::format where suitable and improve exception messages in many places --- libadhocutil/curlHandle.cpp | 15 ++++++++++++ libadhocutil/fileUtils.cpp | 8 +++---- libadhocutil/lexer.cpp | 4 +++- libadhocutil/net.ice | 1 + libadhocutil/plugins.cpp | 16 ++++++++----- libadhocutil/processPipes.cpp | 40 ++++++++++++++------------------ libadhocutil/resourcePool.cpp | 8 ++++--- libadhocutil/runtimeContext.cpp | 6 +++-- libadhocutil/sys.ice | 5 ++++ libadhocutil/sysImpl.cpp | 14 ++++++++--- libadhocutil/unittests/testFileUtils.cpp | 6 +++++ libadhocutil/uriParse.cpp | 6 +++-- 12 files changed, 86 insertions(+), 43 deletions(-) diff --git a/libadhocutil/curlHandle.cpp b/libadhocutil/curlHandle.cpp index b86d8c1..d4d8650 100644 --- a/libadhocutil/curlHandle.cpp +++ b/libadhocutil/curlHandle.cpp @@ -1,6 +1,7 @@ #include "curlHandle.h" #include #include +#include "compileTimeFormatter.h" namespace AdHoc { namespace Net { @@ -120,6 +121,20 @@ CurlHandle::checkCurlCode(CURLcode res) const } } +AdHocFormatter(CurlExceptionMsg, "Network operation failed: %? (%?)"); +AdHocFormatter(CurlExceptionMsgHttp, "HTTP operation failed: %?: %? (%?)"); + +void +CurlException::ice_print(std::ostream & s) const +{ + if (httpcode) { + CurlExceptionMsgHttp::write(s, *httpcode, message, resultcode); + } + else { + CurlExceptionMsg::write(s, message, resultcode); + } +} + } } diff --git a/libadhocutil/fileUtils.cpp b/libadhocutil/fileUtils.cpp index c6acc53..d114d30 100644 --- a/libadhocutil/fileUtils.cpp +++ b/libadhocutil/fileUtils.cpp @@ -16,7 +16,7 @@ namespace AdHoc { fh(open(path.c_str(), flags)) { if (fh < 0) { - throw SystemException("Failed to open " + path.string(), strerror(errno), errno); + throw SystemExceptionOn("open(2) failed", strerror(errno), errno, path.string()); } } @@ -24,7 +24,7 @@ namespace AdHoc { fh(open(path.c_str(), flags, mode)) { if (fh < 0) { - throw SystemException("Failed to open " + path.string(), strerror(errno), errno); + throw SystemExceptionOn("open(2) failed", strerror(errno), errno, path.string()); } } @@ -38,7 +38,7 @@ namespace AdHoc { { if (fstat(fh, &st)) { // LCOV_EXCL_START can't think of a way to test open succeeding and fstat failing - throw SystemException("Failed to fstat " + path.string(), strerror(errno), errno); + throw SystemExceptionOn("fstat(2) failed", strerror(errno), errno, path.string()); // LCOV_EXCL_STOP } } @@ -54,7 +54,7 @@ namespace AdHoc { data(mmap(0, st.st_size, PROT_READ, MAP_SHARED, fh, 0)) { if (data == (void*)-1) { - throw SystemException("Failed to mmap " + path.string(), strerror(errno), errno); + throw SystemExceptionOn("mmap(2) failed", strerror(errno), errno, path.string()); } } diff --git a/libadhocutil/lexer.cpp b/libadhocutil/lexer.cpp index 83d2666..3dd1e0e 100644 --- a/libadhocutil/lexer.cpp +++ b/libadhocutil/lexer.cpp @@ -1,4 +1,5 @@ #include "lexer.h" +#include "compileTimeFormatter.h" namespace AdHoc { const Lexer::State Lexer::InitialState = ""; @@ -11,6 +12,7 @@ namespace AdHoc { { } + AdHocFormatter(UnexpectedInputState, "Unexpected input in state (%?) at %?"); void Lexer::extract(const gchar * string, size_t length) const { @@ -29,7 +31,7 @@ namespace AdHoc { } } if (!selected) { - throw std::runtime_error(std::string("Unexpected input in state (" + es.getState() + ") at ") + (string + es.pos)); + throw std::runtime_error(UnexpectedInputState::get(es.getState(), string + es.pos)); } es.pat = boost::get<1>(*selected); const auto & h = boost::get<2>(*selected); diff --git a/libadhocutil/net.ice b/libadhocutil/net.ice index 510d749..2730e9c 100644 --- a/libadhocutil/net.ice +++ b/libadhocutil/net.ice @@ -1,5 +1,6 @@ module AdHoc { module Net { + ["cpp:ice_print"] exception CurlException { int resultcode; string message; diff --git a/libadhocutil/plugins.cpp b/libadhocutil/plugins.cpp index 2630e57..0435daa 100644 --- a/libadhocutil/plugins.cpp +++ b/libadhocutil/plugins.cpp @@ -3,7 +3,7 @@ #include #include #include -#include "buffer.h" +#include "compileTimeFormatter.h" namespace std { bool @@ -60,24 +60,28 @@ namespace AdHoc { Plugin::~Plugin() = default; + AdHocFormatter(NoSuchPluginExceptionMsg, "No such plugin: %? of type %?"); NoSuchPluginException::NoSuchPluginException(const std::string & n, const std::type_info & t) : - std::runtime_error(stringbf("No such plugin: %s of type %s", n, t)) + std::runtime_error(NoSuchPluginExceptionMsg::get(n, t)) { } + AdHocFormatter(DuplicatePluginExceptionMsg, "Duplicate plugin %? for type %? at %?:%?, originally from %?:%?"); DuplicatePluginException::DuplicatePluginException(PluginPtr p1, PluginPtr p2) : - std::runtime_error(stringbf("Duplicate plugin %s for type %s at %s:%d, originally from %s:%d", - p1->name, p1->type(), p2->filename, p2->lineno, p1->filename, p1->lineno)) + std::runtime_error(DuplicatePluginExceptionMsg::get( + p1->name, p1->type(), p2->filename, p2->lineno, p1->filename, p1->lineno)) { } + AdHocFormatter(DuplicateResolverExceptionMsg, "Duplicate resolver function for type %?"); DuplicateResolverException::DuplicateResolverException(const std::type_info & t) : - std::runtime_error(stringbf("Duplicate resolver function for type %s", t)) + std::runtime_error(DuplicateResolverExceptionMsg::get(t)) { } + AdHocFormatter(LoadLibraryExceptionMsg, "Failed to load library [%?]; %?"); LoadLibraryException::LoadLibraryException(const std::string & f, const char * msg) : - std::runtime_error(stringbf("Failed to load library [%s]; %s", f, msg)) + std::runtime_error(LoadLibraryExceptionMsg::get(f, msg)) { } diff --git a/libadhocutil/processPipes.cpp b/libadhocutil/processPipes.cpp index 511f415..cca7cce 100644 --- a/libadhocutil/processPipes.cpp +++ b/libadhocutil/processPipes.cpp @@ -4,11 +4,25 @@ #include #include #include +#include #include "scopeExit.h" namespace AdHoc { namespace System { +static +void +pipe(int pipes[2], ScopeExit & tidyUp) +{ + if (::pipe(pipes)) { + throw SystemException("pipe(2) failed", strerror(errno), errno); + } + tidyUp.onFailure.push_back([pipes] { + close(pipes[0]); + close(pipes[1]); + }); +} + ProcessPipes::ProcessPipes(const std::vector & args, bool i, bool o, bool e) : in(-1), out(-1), @@ -17,35 +31,17 @@ ProcessPipes::ProcessPipes(const std::vector & args, bool i, bool o int ipipes[2], opipes[2], epipes[2]; ScopeExit tidyUp; if (i) { - if (pipe(ipipes)) { - throw std::runtime_error("Failed to create stdin pipe"); - } - tidyUp.onFailure.push_back([ipipes] { - close(ipipes[0]); - close(ipipes[1]); - }); + pipe(ipipes, tidyUp); } if (o) { - if (pipe(opipes)) { - throw std::runtime_error("Failed to create stdout pipe"); - } - tidyUp.onFailure.push_back([opipes] { - close(opipes[0]); - close(opipes[1]); - }); + pipe(opipes, tidyUp); } if (e) { - if (pipe(epipes)) { - throw std::runtime_error("Failed to create stderr pipe"); - } - tidyUp.onFailure.push_back([epipes] { - close(epipes[0]); - close(epipes[1]); - }); + pipe(epipes, tidyUp); } switch (child = fork()) { case -1: // fail - throw std::runtime_error("Failed to fork"); + throw SystemException("fork(2) failed", strerror(errno), errno); default: // parent if (i) { close(ipipes[0]); diff --git a/libadhocutil/resourcePool.cpp b/libadhocutil/resourcePool.cpp index 29e6cb9..948e419 100644 --- a/libadhocutil/resourcePool.cpp +++ b/libadhocutil/resourcePool.cpp @@ -1,5 +1,5 @@ #include "resourcePool.h" -#include "buffer.h" +#include "compileTimeFormatter.h" namespace AdHoc { TimeOutOnResourcePool::TimeOutOnResourcePool(const char * const n) : @@ -7,10 +7,11 @@ namespace AdHoc { { } + AdHocFormatter(TimeOutOnResourcePoolMsg, "Timeout getting a resource from pool of %?"); std::string TimeOutOnResourcePool::message() const throw() { - return stringbf("Timeout getting a resource from pool of %s", name); + return TimeOutOnResourcePoolMsg::get(name); } NoCurrentResource::NoCurrentResource(const std::thread::id & id, const char * const n) : @@ -19,10 +20,11 @@ namespace AdHoc { { } + AdHocFormatter(NoCurrentResourceMsg, "Thread %? has no current resource handle of type %?"); std::string NoCurrentResource::message() const throw() { - return stringbf("Thread %s has no current resource handle of type %s", threadId, name); + return NoCurrentResourceMsg::get(threadId, name); } } diff --git a/libadhocutil/runtimeContext.cpp b/libadhocutil/runtimeContext.cpp index fc09419..46a67f3 100644 --- a/libadhocutil/runtimeContext.cpp +++ b/libadhocutil/runtimeContext.cpp @@ -1,5 +1,7 @@ #include "runtimeContext.h" -#include +#include +#include +#include namespace AdHoc { namespace System { @@ -10,7 +12,7 @@ RuntimeContext::RuntimeContext(size_t stacksize) : { stack = malloc(stacksize); if (getcontext(&ctxCallback) == -1) - throw std::runtime_error("Failed to getcontext"); + throw SystemException("getcontext(3) failed", strerror(errno), errno); ctxCallback.uc_stack.ss_sp = stack; ctxCallback.uc_stack.ss_size = stacksize; ctxCallback.uc_link = &ctxInitial; diff --git a/libadhocutil/sys.ice b/libadhocutil/sys.ice index 2704aa6..f97fb7f 100644 --- a/libadhocutil/sys.ice +++ b/libadhocutil/sys.ice @@ -5,5 +5,10 @@ module AdHoc { string message; int errNo; }; + + ["cpp:ice_print"] + exception SystemExceptionOn extends SystemException { + string objectName; + }; }; diff --git a/libadhocutil/sysImpl.cpp b/libadhocutil/sysImpl.cpp index 711c098..66ac6ea 100644 --- a/libadhocutil/sysImpl.cpp +++ b/libadhocutil/sysImpl.cpp @@ -1,10 +1,18 @@ #include -#include +#include "compileTimeFormatter.h" namespace AdHoc { - static boost::format e("%s (%d:%s)"); + AdHocFormatter(SystemExceptionMsg, "%? (%?:%?)"); + void SystemException::ice_print(std::ostream & s) const { - s << e % task % errNo % message; + SystemExceptionMsg::write(s, task, errNo, message); + } + + AdHocFormatter(SystemExceptionOnMsg, "%? on '%?' (%?:%?)"); + + void SystemExceptionOn::ice_print(std::ostream & s) const + { + SystemExceptionOnMsg::write(s, task, objectName, errNo, message); } } diff --git a/libadhocutil/unittests/testFileUtils.cpp b/libadhocutil/unittests/testFileUtils.cpp index 00d0a16..2a5ab60 100644 --- a/libadhocutil/unittests/testFileUtils.cpp +++ b/libadhocutil/unittests/testFileUtils.cpp @@ -33,6 +33,12 @@ BOOST_AUTO_TEST_CASE( msg ) BOOST_REQUIRE_EQUAL(x.what(), "verb (2:No such file or directory)"); } +BOOST_AUTO_TEST_CASE( msgOn ) +{ + AdHoc::SystemExceptionOn x("verb", "No such file or directory", ENOENT, "noun"); + BOOST_REQUIRE_EQUAL(x.what(), "verb on 'noun' (2:No such file or directory)"); +} + BOOST_AUTO_TEST_CASE( pathPart ) { using namespace AdHoc::FileUtils; diff --git a/libadhocutil/uriParse.cpp b/libadhocutil/uriParse.cpp index 4a07bdb..4d41937 100644 --- a/libadhocutil/uriParse.cpp +++ b/libadhocutil/uriParse.cpp @@ -1,7 +1,7 @@ #include "uriParse.h" #include #include -#include "buffer.h" +#include "compileTimeFormatter.h" namespace AdHoc { static inline int _is_scheme_char(int c) @@ -179,10 +179,12 @@ namespace AdHoc { { } + AdHocFormatter(InvalidUriMsg, "InvalidUri (%?) parsing [%?]"); + std::string InvalidUri::message() const throw() { - return stringbf("InvalidUri (%s) parsing [%s]", err, uri); + return InvalidUriMsg::get(err, uri); } } -- cgit v1.2.3