Paul J. Lucas has proposed merging lp:~zorba-coders/zorba/bug-1103819 into
lp:zorba.
Commit message:
Reworked some catch clauses.
Requested reviews:
Paul J. Lucas (paul-lucas)
Related bugs:
Bug #1103819 in Zorba: "Use of "catch (...)" should be reduced or mitigated"
https://bugs.launchpad.net/zorba/+bug/1103819
Bug #1106581 in Zorba: "Use of "throw 0" as flow-control"
https://bugs.launchpad.net/zorba/+bug/1106581
Bug #1106586 in Zorba: "Use of "catch (...)" swallows exceptions"
https://bugs.launchpad.net/zorba/+bug/1106586
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug-1103819/+merge/146312
Reworked some catch clauses.
--
https://code.launchpad.net/~zorba-coders/zorba/bug-1103819/+merge/146312
Your team Zorba Coders is subscribed to branch lp:zorba.
=== modified file 'src/api/xqueryimpl.cpp'
--- src/api/xqueryimpl.cpp 2013-01-08 08:34:08 +0000
+++ src/api/xqueryimpl.cpp 2013-02-03 18:00:35 +0000
@@ -79,6 +79,7 @@
namespace zorba
{
+#define QUERY_TRY try
#define QUERY_CATCH \
catch (ZorbaException const& e) \
@@ -941,7 +942,7 @@
{
SYNC_CODE(AutoMutex lock(&theMutex);)
- try
+ QUERY_TRY
{
checkNotClosed();
checkCompiled();
@@ -964,22 +965,7 @@
return true;
}
- catch (ZorbaException const& e)
- {
- ZorbaImpl::notifyError(theDiagnosticHandler, e);
- }
- catch (FlowCtlException const&)
- {
- ZorbaImpl::notifyError(theDiagnosticHandler, "User interrupt");
- }
- catch (std::exception const& e)
- {
- ZorbaImpl::notifyError(theDiagnosticHandler, e.what());
- }
- catch (...)
- {
- ZorbaImpl::notifyError(theDiagnosticHandler);
- }
+ QUERY_CATCH
return false;
}
=== modified file 'src/capi/cdynamic_context.cpp'
--- src/capi/cdynamic_context.cpp 2012-09-19 21:16:15 +0000
+++ src/capi/cdynamic_context.cpp 2013-02-03 18:00:35 +0000
@@ -271,10 +271,7 @@
CDynamicContext::free(XQC_DynamicContext* context)
{
try {
- CDynamicContext* me = CDynamicContext::get(context);
- delete me;
- } catch (ZorbaException const&) {
- assert(false);
+ delete CDynamicContext::get(context);
} catch (...) {
assert(false);
}
=== modified file 'src/capi/cexpression.cpp'
--- src/capi/cexpression.cpp 2012-09-19 21:16:15 +0000
+++ src/capi/cexpression.cpp 2013-02-03 18:00:35 +0000
@@ -227,10 +227,7 @@
CExpression::free(XQC_Expression* expr)
{
try {
- CExpression* me = CExpression::get(expr);
- delete me;
- } catch (ZorbaException const&) {
- assert(false);
+ delete CExpression::get(expr);
} catch (...) {
assert(false);
}
=== modified file 'src/capi/cimplementation.cpp'
--- src/capi/cimplementation.cpp 2012-09-19 21:16:15 +0000
+++ src/capi/cimplementation.cpp 2013-02-03 18:00:35 +0000
@@ -507,10 +507,7 @@
CImplementation::free(XQC_Implementation* impl)
{
try {
- CImplementation* me = CImplementation::get(impl);
- delete me;
- } catch (ZorbaException const&) {
- assert(false);
+ delete CImplementation::get(impl);
} catch (...) {
assert(false);
}
=== modified file 'src/capi/csequence.cpp'
--- src/capi/csequence.cpp 2012-09-19 21:16:15 +0000
+++ src/capi/csequence.cpp 2013-02-03 18:00:35 +0000
@@ -492,10 +492,7 @@
CSequence::free(XQC_Sequence* seq)
{
try {
- CSequence* me = CSequence::get(seq);
- delete me;
- } catch (ZorbaException const&) {
- assert(false);
+ delete CSequence::get(seq);
} catch (...) {
assert(false);
}
=== modified file 'src/capi/cstatic_context.cpp'
--- src/capi/cstatic_context.cpp 2012-09-19 21:16:15 +0000
+++ src/capi/cstatic_context.cpp 2013-02-03 18:00:35 +0000
@@ -666,10 +666,7 @@
CStaticContext::free(XQC_StaticContext* context)
{
try {
- CStaticContext* me = CStaticContext::get(context);
- delete me;
- } catch (ZorbaException const&) {
- assert(false);
+ delete CStaticContext::get(context);
} catch (...) {
assert(false);
}
=== modified file 'src/context/default_url_resolvers.cpp'
--- src/context/default_url_resolvers.cpp 2013-01-24 12:11:43 +0000
+++ src/context/default_url_resolvers.cpp 2013-02-03 18:00:35 +0000
@@ -21,6 +21,7 @@
#include "util/uri_util.h"
#include "util/http_util.h"
#include "util/fs_util.h"
+#include "util/string_util.h"
#include "store/api/store.h"
#include "store/api/item_factory.h"
#include "store/api/collection.h"
@@ -93,6 +94,11 @@
new StreamResource(&(lStream->getStream()), lStream->getStreamReleaser());
lStream->setStreamReleaser(nullptr);
return lResource;
+ } catch (std::exception const &e) {
+ std::string const msg(
+ BUILD_STRING( "Could not create stream resource: ", e.what() )
+ );
+ throw os_error::exception( "", aUrl.c_str(), msg.c_str() );
} catch (...) {
throw os_error::exception("", aUrl.c_str(), "Could not create stream resource");
}
=== modified file 'src/runtime/debug/debug_iterator_impl.cpp'
--- src/runtime/debug/debug_iterator_impl.cpp 2012-09-19 21:16:15 +0000
+++ src/runtime/debug/debug_iterator_impl.cpp 2013-02-03 18:00:35 +0000
@@ -205,25 +205,18 @@
if (lCommons->canBreak() && (lCommons->mustBreak(lCause) ||
lCommons->hasToBreakAt(loc) || lCommons->hasToBreakAt(this)))
{
- try
- {
- lCause = lCause == 0 ? CAUSE_BREAKPOINT : lCause;
-
- // tell everybody that we are the iterator who suspended
- //lCommons->setCurrentIterator(this);
- lCommons->setCurrentStaticContext(getStaticContext());
- lCommons->setCurrentDynamicContext(planState->theLocalDynCtx);
- lCommons->setBreak(false);
- lCommons->setPlanState(planState);
+ lCause = lCause == 0 ? CAUSE_BREAKPOINT : lCause;
+
+ // tell everybody that we are the iterator who suspended
+ //lCommons->setCurrentIterator(this);
+ lCommons->setCurrentStaticContext(getStaticContext());
+ lCommons->setCurrentDynamicContext(planState->theLocalDynCtx);
+ lCommons->setBreak(false);
+ lCommons->setPlanState(planState);
- // suspend
- lCommons->getRuntime()->suspendRuntime(loc, lCause);
- lCause = 0;
- }
- catch (...)
- {
- throw;
- }
+ // suspend
+ lCommons->getRuntime()->suspendRuntime(loc, lCause);
+ lCause = 0;
}
}
#endif // ZORBA_WITH_DEBUGGER
=== modified file 'src/runtime/indexing/doc_indexer.cpp'
--- src/runtime/indexing/doc_indexer.cpp 2012-09-19 21:16:15 +0000
+++ src/runtime/indexing/doc_indexer.cpp 2013-02-03 18:00:35 +0000
@@ -150,9 +150,7 @@
}
catch(...)
{
- if (key != NULL)
- delete key;
-
+ delete key;
throw;
}
}
=== modified file 'src/store/naive/loader_dtd.cpp'
--- src/store/naive/loader_dtd.cpp 2013-01-30 02:18:08 +0000
+++ src/store/naive/loader_dtd.cpp 2013-02-03 18:00:35 +0000
@@ -384,6 +384,9 @@
}
catch (...)
{
+ // catch-TODO: "throw 0" and "catch(...)" are being used here as "flow
+ // control" -- not an appropriate use of C++ exceptions. The catch here
+ // will silently swallow all other exceptions as well -- not good.
abortload();
thePathStack.clear();
return NULL;
@@ -760,6 +763,14 @@
)
);
}
+ catch (std::exception const &e)
+ {
+ theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZSTR0020_LOADER_IO_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
theXQueryDiagnostics->add_error(
@@ -1092,6 +1103,14 @@
{
loader.theXQueryDiagnostics->add_error( e );
}
+ catch (std::exception const &e)
+ {
+ loader.theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
loader.theXQueryDiagnostics->add_error(
@@ -1174,6 +1193,14 @@
{
loader.theXQueryDiagnostics->add_error( e );
}
+ catch (std::exception const &e)
+ {
+ loader.theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
loader.theXQueryDiagnostics->add_error(
@@ -1468,6 +1495,14 @@
{
loader.theXQueryDiagnostics->add_error( e );
}
+ catch (std::exception const &e)
+ {
+ loader.theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
loader.theXQueryDiagnostics->
@@ -1600,6 +1635,14 @@
{
loader.theXQueryDiagnostics->add_error( e );
}
+ catch (std::exception const &e)
+ {
+ loader.theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
loader.theXQueryDiagnostics->add_error(
@@ -1651,6 +1694,14 @@
{
loader.theXQueryDiagnostics->add_error( e );
}
+ catch (std::exception const &e)
+ {
+ loader.theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
loader.theXQueryDiagnostics->add_error(
@@ -1704,6 +1755,14 @@
{
loader.theXQueryDiagnostics->add_error( e );
}
+ catch (std::exception const &e)
+ {
+ loader.theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
loader.theXQueryDiagnostics->add_error(
@@ -1754,6 +1813,14 @@
{
loader.theXQueryDiagnostics->add_error( e );
}
+ catch (std::exception const &e)
+ {
+ loader.theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
loader.theXQueryDiagnostics->add_error(
@@ -1797,6 +1864,14 @@
{
loader.theXQueryDiagnostics->add_error( e );
}
+ catch (std::exception const &e)
+ {
+ loader.theXQueryDiagnostics->add_error(
+ NEW_ZORBA_EXCEPTION(
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() )
+ )
+ );
+ }
catch (...)
{
loader.theXQueryDiagnostics->add_error(
=== modified file 'src/store/naive/loader_fast.cpp'
--- src/store/naive/loader_fast.cpp 2013-01-21 03:22:59 +0000
+++ src/store/naive/loader_fast.cpp 2013-02-03 18:00:35 +0000
@@ -71,6 +71,25 @@
return; \
} while (0);
+#define LOADER_TRY try
+
+#define LOADER_CATCH \
+ catch ( ZorbaException const &e ) { \
+ loader.theXQueryDiagnostics->add_error( e ); \
+ } \
+ catch ( std::exception const &e ) { \
+ loader.theXQueryDiagnostics->add_error( \
+ NEW_ZORBA_EXCEPTION( \
+ zerr::ZXQP0003_INTERNAL_ERROR, ERROR_PARAMS( e.what() ) \
+ ) \
+ ); \
+ } \
+ catch ( ... ) { \
+ loader.theXQueryDiagnostics->add_error( \
+ NEW_ZORBA_EXCEPTION( zerr::ZXQP0003_INTERNAL_ERROR ) \
+ ); \
+ }
+
/*******************************************************************************
@@ -327,7 +346,8 @@
return stream.gcount();
}
- catch (std::iostream::failure e)
+ // catch-TODO: should ZorbaException be caught seperately here?
+ catch (std::exception const &e)
{
theXQueryDiagnostics->add_error(
NEW_ZORBA_EXCEPTION(
@@ -443,7 +463,7 @@
catch(...)
{
abortload();
- return NULL;
+ return NULL; // catch-TODO: "throw;" here?
}
bool ok = ctxt->wellFormed != 0;
@@ -498,7 +518,7 @@
FastXmlLoader& loader = *(static_cast<FastXmlLoader *>(ctx));
ZORBA_LOADER_CHECK_ERROR(loader);
- try
+ LOADER_TRY
{
DocumentNode* docNode = GET_STORE().getNodeFactory().createDocumentNode();
@@ -523,15 +543,7 @@
LOADER_TRACE1("Start Doc Node = " << docNode);
}
- catch (ZorbaException const& e)
- {
- loader.theXQueryDiagnostics->add_error( e );
- }
- catch (...)
- {
- loader.theXQueryDiagnostics->
- add_error(NEW_ZORBA_EXCEPTION(zerr::ZXQP0003_INTERNAL_ERROR));
- }
+ LOADER_CATCH
}
@@ -554,7 +566,7 @@
DocumentNode* docNode;
XmlNode* currChild;
- try
+ LOADER_TRY
{
// This check is required because it is possible (in case of mal-formed doc)
// that libXml calls endDocument() without having called startDocument().
@@ -608,16 +620,7 @@
LOADER_TRACE2("End Doc Node = " << docNode);
}
- catch (ZorbaException const& e)
- {
- loader.theXQueryDiagnostics->add_error( e );
- }
- catch (...)
- {
- loader.theXQueryDiagnostics->add_error(
- NEW_ZORBA_EXCEPTION( zerr::ZXQP0003_INTERNAL_ERROR )
- );
- }
+ LOADER_CATCH
}
@@ -658,7 +661,7 @@
zorba::Stack<PathStepInfo>& pathStack = loader.thePathStack;
zstring baseUri;
- try
+ LOADER_TRY
{
csize numAttributes = static_cast<csize>(numAttrs);
csize numBindings = static_cast<csize>(numNamespaces);
@@ -849,15 +852,7 @@
nodeStack.push(NULL);
pathStack.push(PathStepInfo(elemNode, baseUri));
}
- catch (ZorbaException const& e)
- {
- loader.theXQueryDiagnostics->add_error( e );
- }
- catch (...)
- {
- loader.theXQueryDiagnostics->
- add_error(NEW_ZORBA_EXCEPTION(zerr::ZXQP0003_INTERNAL_ERROR));
- }
+ LOADER_CATCH
}
@@ -890,7 +885,7 @@
XmlNode* prevChild = NULL;
XmlNode* currChild;
- try
+ LOADER_TRY
{
// Find the position of the 1st child of this element node in the node stack
firstChildPos = stackSize - 1;
@@ -977,16 +972,7 @@
}
#endif
}
- catch (ZorbaException const& e)
- {
- loader.theXQueryDiagnostics->add_error( e );
- }
- catch (...)
- {
- loader.theXQueryDiagnostics->add_error(
- NEW_ZORBA_EXCEPTION( zerr::ZXQP0003_INTERNAL_ERROR )
- );
- }
+ LOADER_CATCH
}
@@ -1002,7 +988,7 @@
FastXmlLoader& loader = *(static_cast<FastXmlLoader *>( ctx ));
ZORBA_LOADER_CHECK_ERROR(loader);
- try
+ LOADER_TRY
{
const char* charp = reinterpret_cast<const char*>(ch);
zstring content(charp, len);
@@ -1028,16 +1014,7 @@
LOADER_TRACE2(lSs.str());
#endif
}
- catch (ZorbaException const& e)
- {
- loader.theXQueryDiagnostics->add_error( e );
- }
- catch (...)
- {
- loader.theXQueryDiagnostics->add_error(
- NEW_ZORBA_EXCEPTION( zerr::ZXQP0003_INTERNAL_ERROR )
- );
- }
+ LOADER_CATCH
}
@@ -1053,7 +1030,7 @@
FastXmlLoader& loader = *(static_cast<FastXmlLoader *>( ctx ));
ZORBA_LOADER_CHECK_ERROR(loader);
- try
+ LOADER_TRY
{
// If a doc contains an element like <cdata><![CDATA[ <> ]]></cdata>,
// libxml returns the string " <> ".
@@ -1081,16 +1058,7 @@
LOADER_TRACE2(lSs.str());
#endif
}
- catch (ZorbaException const& e)
- {
- loader.theXQueryDiagnostics->add_error( e );
- }
- catch (...)
- {
- loader.theXQueryDiagnostics->add_error(
- NEW_ZORBA_EXCEPTION( zerr::ZXQP0003_INTERNAL_ERROR )
- );
- }
+ LOADER_CATCH
}
@@ -1108,7 +1076,7 @@
FastXmlLoader& loader = *(static_cast<FastXmlLoader *>( ctx ));
ZORBA_LOADER_CHECK_ERROR(loader);
- try
+ LOADER_TRY
{
// bugfix: handling PIs with no data (i.e. data being NULL)
zstring content;
@@ -1131,16 +1099,7 @@
<< targetp << std::endl << " ordpath = "
<< piNode->getOrdPath().show() << std::endl);
}
- catch (ZorbaException const& e)
- {
- loader.theXQueryDiagnostics->add_error( e );
- }
- catch (...)
- {
- loader.theXQueryDiagnostics->add_error(
- NEW_ZORBA_EXCEPTION( zerr::ZXQP0003_INTERNAL_ERROR )
- );
- }
+ LOADER_CATCH
}
@@ -1155,7 +1114,7 @@
FastXmlLoader& loader = *(static_cast<FastXmlLoader *>( ctx ));
ZORBA_LOADER_CHECK_ERROR(loader);
- try
+ LOADER_TRY
{
const char* charp = reinterpret_cast<const char*>(ch);
zstring content;
@@ -1176,16 +1135,7 @@
<< charp << std::endl << " ordpath = "
<< commentNode->getOrdPath().show() << std::endl);
}
- catch (ZorbaException const& e)
- {
- loader.theXQueryDiagnostics->add_error( e );
- }
- catch (...)
- {
- loader.theXQueryDiagnostics->add_error(
- NEW_ZORBA_EXCEPTION( zerr::ZXQP0003_INTERNAL_ERROR )
- );
- }
+ LOADER_CATCH
}
=== modified file 'src/store/naive/node_items.cpp'
--- src/store/naive/node_items.cpp 2013-01-11 00:45:15 +0000
+++ src/store/naive/node_items.cpp 2013-02-03 18:00:35 +0000
@@ -751,7 +751,11 @@
return (pos < numChildren);
}
}
- catch(...)
+ catch (std::exception const &e)
+ {
+ ZORBA_FATAL(false, e.what());
+ }
+ catch (...)
{
ZORBA_FATAL(false, "Unexpected exception");
}
@@ -825,6 +829,10 @@
destroyInternal(removeType);
}
+ catch (std::exception const &e)
+ {
+ ZORBA_FATAL(false, e.what());
+ }
catch (...)
{
ZORBA_FATAL(false, "Unexpected exception");
=== modified file 'src/store/naive/node_updates.cpp'
--- src/store/naive/node_updates.cpp 2012-09-19 21:16:15 +0000
+++ src/store/naive/node_updates.cpp 2013-02-03 18:00:35 +0000
@@ -519,6 +519,10 @@
SYNC_CODE(oldTree->getRCLock()->release());
}
}
+ catch (std::exception const &e)
+ {
+ ZORBA_FATAL(false, e.what());
+ }
catch(...)
{
ZORBA_FATAL(0, "Unexpected exception");
=== modified file 'src/store/naive/pul_primitives.cpp'
--- src/store/naive/pul_primitives.cpp 2013-01-22 11:07:53 +0000
+++ src/store/naive/pul_primitives.cpp 2013-02-03 18:00:35 +0000
@@ -743,9 +743,13 @@
{
theRevalidationPul->applyUpdates(false);
}
+ catch (std::exception const &e)
+ {
+ ZORBA_FATAL(false, "Error during the in-place validation: " << e.what());
+ }
catch (...)
{
- ZORBA_FATAL(0, "Error during the in-place validation");
+ ZORBA_FATAL(false, "Error during the in-place validation");
}
theIsApplied = true;
=== modified file 'src/store/naive/store.cpp'
--- src/store/naive/store.cpp 2012-11-21 03:17:34 +0000
+++ src/store/naive/store.cpp 2013-02-03 18:00:35 +0000
@@ -574,9 +574,7 @@
}
catch(...)
{
- if (key != NULL)
- delete key;
-
+ delete key;
sourceIter->close();
throw;
}
@@ -1050,6 +1048,7 @@
catch(...)
{
delete stream;
+ // catch-TODO: "throw;" here?
}
return docitem;
}
=== modified file 'src/types/schema/schema.cpp'
--- src/types/schema/schema.cpp 2013-01-24 10:49:06 +0000
+++ src/types/schema/schema.cpp 2013-02-03 18:00:35 +0000
@@ -488,7 +488,7 @@
ERROR_PARAMS( std::string(xsdURL), ZED( SchemaParseError ), StrX(e.getMessage())),
ERROR_LOC( loc ) );
}
- catch (const ZorbaException& /* e */)
+ catch (const ZorbaException&)
{
throw;
}
@@ -1763,18 +1763,6 @@
targetType->toSchemaString(),
msg));
}
- catch(const OutOfMemoryException&)
- {
- throw;
- }
- catch (const ZorbaException&)
- {
- throw;
- }
- catch (...)
- {
- throw;
- }
#endif //ZORBA_NO_XMLSCHEMA
// find the non user defined base type
@@ -2079,6 +2067,7 @@
}
catch (...)
{
+ // catch-TODO: should all exceptions really be ignored here?
}
ar & binstr;
--
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help : https://help.launchpad.net/ListHelp
