Author: brane
Date: Fri Jul 26 19:47:27 2013
New Revision: 1507413
URL: http://svn.apache.org/r1507413
Log:
Refactor svn_error_t wrapping in C++HL so that we do not have to expose the
type name in the public API, and simplify memory management.
* subversion/bindings/cxxhl/include/svncxxhl/exception.hpp
(InternalError::InternalError): Make constructors explicit.
(InternalError::m_description): Change type to a shared_ptr.
(Error::Error): Remove public constructors and make constructor explicit.
(Error::smart_ptr, Error::nested, Error::throw_svn_error,
Error::m_errno, Error::m_nested): Remove.
(Error::Message): Make this a nested class instead of using std::pair.
(Cancelled::Cancelled): Make constructor explicit.
* subversion/bindings/cxxhl/src/cxxhl-private.hpp: New header.
* subversion/bindings/cxxhl/src/private/exception.hpp: New header.
(detail::checked_call): New prototype. Replaces Error::throw_svn_error.
* subversion/bindings/cxxhl/src/exception.cpp: Refactor InternalError,
Error and Cancelled implementation. Subversion error stack is now
represented by chained error descriptions instead of chained
exception objects.
(detail::checked_call): Implement.
* subversion/bindings/cxxhl/tests/test_exception.cpp:
Use new Error::Message and detail::checked_call functions.
Added:
subversion/trunk/subversion/bindings/cxxhl/src/cxxhl-private.hpp (with
props)
subversion/trunk/subversion/bindings/cxxhl/src/private/
subversion/trunk/subversion/bindings/cxxhl/src/private/exception.hpp
(with props)
Modified:
subversion/trunk/subversion/bindings/cxxhl/include/svncxxhl/exception.hpp
subversion/trunk/subversion/bindings/cxxhl/src/exception.cpp
subversion/trunk/subversion/bindings/cxxhl/tests/test_exception.cpp
Modified:
subversion/trunk/subversion/bindings/cxxhl/include/svncxxhl/exception.hpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxxhl/include/svncxxhl/exception.hpp?rev=1507413&r1=1507412&r2=1507413&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxxhl/include/svncxxhl/exception.hpp
(original)
+++ subversion/trunk/subversion/bindings/cxxhl/include/svncxxhl/exception.hpp
Fri Jul 26 19:47:27 2013
@@ -35,9 +35,6 @@
#include "svncxxhl/_compat.hpp"
-// Forward declaration of implementation-specific structure
-struct svn_error_t;
-
namespace apache {
namespace subversion {
namespace cxxhl {
@@ -55,7 +52,7 @@ class ErrorDescription;
class InternalError : public std::exception
{
public:
- InternalError(const char* description);
+ explicit InternalError(const char* description);
InternalError(const InternalError& that) throw();
InternalError& operator= (const InternalError& that) throw();
@@ -67,10 +64,9 @@ public:
virtual const char* what() const throw();
protected:
- InternalError(detail::ErrorDescription* description) throw();
-
- /** Error description and trace location information. */
- detail::ErrorDescription* m_description;
+ typedef compat::shared_ptr<detail::ErrorDescription> description_ptr;
+ explicit InternalError(description_ptr description) throw();
+ description_ptr m_description;
};
/**
@@ -79,34 +75,66 @@ protected:
class Error : public InternalError
{
public:
- typedef compat::shared_ptr<Error> shared_ptr;
-
- Error(const char* description, int error_code);
- Error(const char* description, int error_code, shared_ptr nested_error);
-
Error(const Error& that) throw();
Error& operator=(const Error& that) throw();
virtual ~Error() throw();
/**
- * Returns the error code associated with the exception.
- */
- virtual int code() const throw() { return m_errno; }
-
- /**
- * Returns a shared pointer to the nested exception object, if any.
+ * Returns the error code associated with the top-level error that
+ * caused the exception.
*/
- virtual shared_ptr nested() const throw() { return m_nested; }
+ virtual int code() const throw();
/**
* Error message description.
- *
- * The first element of this pair is the error code, the second the
- * associated error message. If the error code is 0, the message
- * describes the location in the source code where the error was
- * generated from.
*/
- typedef std::pair<int, std::string> Message;
+ class Message
+ {
+ public:
+ /**
+ * Create a message object given an error code and error message.
+ */
+ Message(int errno, const std::string& message)
+ : m_errno(errno),
+ m_message(message),
+ m_trace(false)
+ {}
+
+ /**
+ * Create a message object given an error code and error message,
+ * and set the flag that tells if this is a debugging traceback entry.
+ */
+ Message(int errno, const std::string& message, bool trace)
+ : m_errno(errno),
+ m_message(message),
+ m_trace(trace)
+ {}
+
+ /**
+ * Return the error code.
+ */
+ int code() const throw() { return m_errno; }
+
+ /**
+ * Return the error message.
+ */
+ const std::string& message() const throw() { return m_message; }
+
+ /**
+ * Return the generic error message associated with the error code.
+ */
+ const char* generic_message() const;
+
+ /**
+ * Check if this message is in fact a debugging traceback entry.
+ */
+ bool trace() const throw() { return m_trace; }
+
+ private:
+ int m_errno;
+ std::string m_message;
+ bool m_trace;
+ };
/**
* The list of messages associated with an error.
@@ -115,7 +143,7 @@ public:
/**
* Returns the complete list of error messages, including those from
- * nested exceptions.
+ * nested errors.
*/
virtual MessageList messages() const
{
@@ -134,18 +162,11 @@ public:
return compile_messages(true);
}
-public:
- /** Used internally by the implementation. */
- static void throw_svn_error(svn_error_t*);
-
protected:
- Error(int error_code, detail::ErrorDescription* description) throw();
-
-private:
+ explicit Error(description_ptr description) throw()
+ : InternalError(description)
+ {}
MessageList compile_messages(bool show_traces) const;
-
- int m_errno; /**< The (SVN or APR) error code. */
- shared_ptr m_nested; /**< Optional pointer to nessted error. */
};
/**
@@ -155,9 +176,8 @@ private:
class Cancelled : public Error
{
protected:
- friend void Error::throw_svn_error(svn_error_t*);
- Cancelled(int error_code, detail::ErrorDescription* description) throw()
- : Error(error_code, description)
+ explicit Cancelled(description_ptr description) throw()
+ : Error(description)
{}
};
Added: subversion/trunk/subversion/bindings/cxxhl/src/cxxhl-private.hpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxxhl/src/cxxhl-private.hpp?rev=1507413&view=auto
==============================================================================
--- subversion/trunk/subversion/bindings/cxxhl/src/cxxhl-private.hpp (added)
+++ subversion/trunk/subversion/bindings/cxxhl/src/cxxhl-private.hpp Fri Jul 26
19:47:27 2013
@@ -0,0 +1,29 @@
+/**
+ * @copyright
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ * @endcopyright
+ */
+
+#ifndef SVN_CXXHL_PRIVATE_PRIVATE_H
+#define SVN_CXXHL_PRIVATE_PRIVATE_H
+
+#include "private/exception.hpp"
+
+#endif // SVN_CXXHL_PRIVATE_PRIVATE_H
Propchange: subversion/trunk/subversion/bindings/cxxhl/src/cxxhl-private.hpp
------------------------------------------------------------------------------
svn:eol-style = native
Modified: subversion/trunk/subversion/bindings/cxxhl/src/exception.cpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxxhl/src/exception.cpp?rev=1507413&r1=1507412&r2=1507413&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxxhl/src/exception.cpp (original)
+++ subversion/trunk/subversion/bindings/cxxhl/src/exception.cpp Fri Jul 26
19:47:27 2013
@@ -29,6 +29,7 @@
#include "svncxxhl/exception.hpp"
#include "aprwrap.hpp"
+#include "cxxhl-private.hpp"
#include "svn_error.h"
#include "svn_utf.h"
@@ -47,69 +48,59 @@ namespace detail {
class ErrorDescription
{
public:
- static ErrorDescription* create(const char* message,
- const char *loc_file, long loc_line,
- bool trace_link)
+ typedef compat::shared_ptr<ErrorDescription> shared_ptr;
+
+ static shared_ptr create(const char* message, int error_code,
+ const char *loc_file, long loc_line,
+ bool trace_link)
{
- bool empty_message = (message == NULL);
+ const bool empty_message = (message == NULL);
const std::size_t length = (empty_message ? 0 : std::strlen(message));
- void *memblock = ::operator new(length + sizeof(ErrorDescription));
+ void* memblock = ::operator new(length + sizeof(ErrorDescription));
ErrorDescription* description = new(memblock) ErrorDescription(
- loc_file, loc_line, trace_link, empty_message);
+ error_code, loc_file, loc_line, trace_link, empty_message);
if (length)
std::memcpy(description->m_message, message, length);
description->m_message[length] = 0;
- return description;
- }
-
- static ErrorDescription* create(const char* message)
- {
- return create(message, NULL, 0, false);
+ return shared_ptr(description);
}
- ErrorDescription* reference() throw()
+ static shared_ptr create(const char* message, int error_code)
{
- if (this)
- svn_atomic_inc(&m_refcount);
- return this;
+ return create(message, error_code, NULL, 0, false);
}
- ErrorDescription* dereference() throw()
- {
- if (this && 0 == svn_atomic_dec(&m_refcount))
- {
- this->~ErrorDescription();
- ::operator delete(this, std::nothrow);
- return NULL;
- }
- return this;
- }
+ ~ErrorDescription() throw() {}
const char* what() const throw() { return (m_empty ? NULL : m_message); }
+ int code() const throw() { return m_errno; }
const char* file() const throw() { return m_loc_file; }
long line() const throw() { return m_loc_line; }
bool trace() const throw() { return m_trace; }
+ shared_ptr& nested() throw() { return m_nested; }
+ const shared_ptr& nested() const throw() { return m_nested; }
private:
- ErrorDescription(const char *loc_file, long loc_line,
+ ErrorDescription(int error_code,
+ const char *loc_file, long loc_line,
bool trace_link, bool empty_message) throw()
: m_loc_file(loc_file),
m_loc_line(loc_line),
m_trace(trace_link),
m_empty(empty_message),
- m_refcount(0)
+ m_errno(error_code)
{}
- ~ErrorDescription() throw() {}
-
const char* m_loc_file;
long m_loc_line;
bool m_trace;
bool m_empty;
- volatile svn_atomic_t m_refcount;
- char m_message[1];
+ shared_ptr m_nested;
+
+ int m_errno; ///< The (SVN or APR) error code.
+ char m_message[1]; ///< The error message
};
} // namespace detail
@@ -119,11 +110,11 @@ private:
//
InternalError::InternalError(const char* description)
- : m_description(detail::ErrorDescription::create(description)->reference())
+ : m_description(detail::ErrorDescription::create(description, 0))
{}
InternalError::InternalError(const InternalError& that) throw()
- : m_description(that.m_description->reference())
+ : m_description(that.m_description)
{}
InternalError& InternalError::operator=(const InternalError& that) throw()
@@ -138,17 +129,14 @@ InternalError& InternalError::operator=(
return *new(this) InternalError(that);
}
-InternalError::~InternalError() throw()
-{
- m_description->dereference();
-}
+InternalError::~InternalError() throw() {}
const char* InternalError::what() const throw()
{
return m_description->what();
}
-InternalError::InternalError(detail::ErrorDescription* description) throw()
+InternalError::InternalError(description_ptr description) throw()
: m_description(description)
{}
@@ -156,22 +144,8 @@ InternalError::InternalError(detail::Err
// Class Error
//
-Error::Error(const char* description, int error_code)
- : InternalError(description),
- m_errno(error_code)
-{}
-
-Error::Error(const char* description, int error_code,
- Error::shared_ptr nested_error)
- : InternalError(description),
- m_errno(error_code),
- m_nested(nested_error)
-{}
-
Error::Error(const Error& that) throw()
- : InternalError(that.m_description->reference()),
- m_errno(that.m_errno),
- m_nested(that.m_nested)
+ : InternalError(that.m_description)
{}
Error& Error::operator=(const Error& that) throw()
@@ -188,72 +162,43 @@ Error& Error::operator=(const Error& tha
Error::~Error() throw() {}
-Error::Error(int error_code, detail::ErrorDescription* description) throw()
- : InternalError(description),
- m_errno(error_code)
-{}
-
-void Error::throw_svn_error(svn_error_t* err)
+int Error::code() const throw()
{
- detail::ErrorDescription* description = NULL;
- try
- {
- // Be very careful when creating the error descriptions, so that
- // the exception unwinder can free them if an allocation fails.
- // The private constructor does not increment the refcount
- // precisely for this reason.
+ return m_description->code();
+}
- shared_ptr nested;
- shared_ptr* current = &nested;
+namespace {
+const char* get_generic_message(apr_status_t error_code,
+ const APR::Pool& scratch_pool)
+{
+ char errorbuf[512];
- bool cancelled = (err->apr_err == SVN_ERR_CANCELLED);
- for (svn_error_t* next = err->child; next; next = next->child)
- {
- description = detail::ErrorDescription::create(
- next->message, next->file, next->line,
- svn_error__is_tracing_link(next));
- description->reference();
- current->reset(new Error(next->apr_err, description));
- description = NULL;
- current = &(*current)->m_nested;
- if (next->apr_err == SVN_ERR_CANCELLED)
- cancelled = true;
- }
+ // Is this a Subversion-specific error code?
+ if (error_code > APR_OS_START_USEERR && error_code <= APR_OS_START_CANONERR)
+ return svn_strerror(error_code, errorbuf, sizeof(errorbuf));
+ // Otherwise, this must be an APR error code.
+ else
+ {
+ const char* generic;
+ svn_error_t* err = svn_utf_cstring_to_utf8(
+ &generic,
+ apr_strerror(error_code, errorbuf, sizeof(errorbuf)),
+ scratch_pool.get());
+ if (!err)
+ return generic;
- const int apr_err = err->apr_err;
- description = detail::ErrorDescription::create(
- err->message, err->file, err->line,
- svn_error__is_tracing_link(err));
- description->reference();
+ // Use fuzzy transliteration instead.
svn_error_clear(err);
- if (cancelled)
- {
- Cancelled converted = Cancelled(apr_err, description);
- description = NULL;
- converted.m_nested = nested;
- throw converted;
- }
- else
- {
- Error converted = Error(apr_err, description);
- description = NULL;
- converted.m_nested = nested;
- throw converted;
- }
- }
- catch (...)
- {
- description->dereference();
- throw;
+ return svn_utf_cstring_from_utf8_fuzzy(errorbuf, scratch_pool.get());
}
}
-
-namespace {
void handle_one_error(Error::MessageList& ml, bool show_traces,
- int error_code, detail::ErrorDescription* descr,
+ const detail::ErrorDescription* descr,
const APR::Pool& pool)
{
+ const int error_code = descr->code();
+
if (show_traces && descr->file())
{
const char* file_utf8 = NULL;
@@ -274,15 +219,13 @@ void handle_one_error(Error::MessageList
else
{
#ifdef SVN_DEBUG
- if (const char *const symbolic_name =
- svn_error_symbolic_name(error_code))
- //if (symbolic_name)
+ if (const char* symbolic_name = svn_error_symbolic_name(error_code))
buffer << ": (apr_err=" << symbolic_name << ')';
else
#endif
buffer << ": (apr_err=" << error_code << ')';
}
- ml.push_back(Error::Message(0, buffer.str()));
+ ml.push_back(Error::Message(error_code, buffer.str(), true));
}
if (descr->trace())
@@ -290,28 +233,8 @@ void handle_one_error(Error::MessageList
const char *description = descr->what();
if (!description)
- {
- char errorbuf[512];
-
- // Is this a Subversion-specific error code?
- if (error_code > APR_OS_START_USEERR
- && error_code <= APR_OS_START_CANONERR)
- description = svn_strerror(error_code, errorbuf, sizeof(errorbuf));
- // Otherwise, this must be an APR error code.
- else
- {
- svn_error_t* err = svn_utf_cstring_to_utf8(
- &description,
- apr_strerror(error_code, errorbuf, sizeof(errorbuf)),
- pool.get());
- if (err)
- {
- svn_error_clear(err);
- description = _("Can't recode error string from APR");
- }
- }
- }
- ml.push_back(Error::Message(error_code, std::string(description)));
+ description = get_generic_message(error_code, pool);
+ ml.push_back(Error::Message(error_code, std::string(description), false));
}
} // anonymous namespace
@@ -319,11 +242,12 @@ Error::MessageList Error::compile_messag
{
// Determine the maximum size of the returned list
MessageList::size_type max_length = 0;
- for (const Error* err = this; err; err = err->m_nested.get())
+ for (const detail::ErrorDescription* description = m_description.get();
+ description; description = description->nested().get())
{
- if (show_traces && m_description->file())
+ if (show_traces && description->file())
++max_length; // We will display an error location
- if (!m_description->trace())
+ if (!description->trace())
++max_length; // Traces do not emit a message line
}
MessageList ml;
@@ -335,26 +259,69 @@ Error::MessageList Error::compile_messag
empties.reserve(max_length);
APR::IterationPool iterbase;
- for (const Error* err = this; err; err = err->m_nested.get())
+ for (const detail::ErrorDescription* description = m_description.get();
+ description; description = description->nested().get())
{
APR::Pool::Iteration iterpool(iterbase);
- if (!err->m_description->what())
+ if (!description->what())
{
// Non-specific messages are printed only once.
std::vector<int>::iterator it = std::find(
- empties.begin(), empties.end(), err->m_errno);
+ empties.begin(), empties.end(), description->code());
if (it != empties.end())
continue;
- empties.push_back(err->m_errno);
+ empties.push_back(description->code());
}
- handle_one_error(ml, show_traces,
- err->m_errno, err->m_description,
- iterpool.pool());
+ handle_one_error(ml, show_traces, description, iterpool.pool());
}
return ml;
}
+const char* Error::Message::generic_message() const
+{
+ APR::Pool pool;
+ return get_generic_message(m_errno, pool);
+}
+
+namespace detail {
+void checked_call(svn_error_t* err)
+{
+ if (!err)
+ return;
+
+ struct ErrorBuilder : public Cancelled
+ {
+ explicit ErrorBuilder (ErrorDescription::shared_ptr description)
+ : Cancelled(description)
+ {}
+
+ Error error() { return static_cast<Error>(*this); }
+ Error cancelled() { return static_cast<Cancelled>(*this); }
+ };
+
+ ErrorDescription::shared_ptr description;
+ ErrorDescription::shared_ptr* current = &description;
+
+ bool cancelled = false;
+ for (svn_error_t* next = err; next; next = next->child)
+ {
+ *current = ErrorDescription::create(next->message, next->apr_err,
+ next->file, next->line,
+ svn_error__is_tracing_link(next));
+ current = &(*current)->nested();
+ if (next->apr_err == SVN_ERR_CANCELLED)
+ cancelled = true;
+ }
+ svn_error_clear(err);
+
+ if (cancelled)
+ throw ErrorBuilder(description).cancelled();
+ else
+ throw ErrorBuilder(description).error();
+}
+} // namespace detail
+
} // namespace cxxhl
} // namespace subversion
} // namespace apache
Added: subversion/trunk/subversion/bindings/cxxhl/src/private/exception.hpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxxhl/src/private/exception.hpp?rev=1507413&view=auto
==============================================================================
--- subversion/trunk/subversion/bindings/cxxhl/src/private/exception.hpp (added)
+++ subversion/trunk/subversion/bindings/cxxhl/src/private/exception.hpp Fri
Jul 26 19:47:27 2013
@@ -0,0 +1,49 @@
+/**
+ * @copyright
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ * @endcopyright
+ */
+
+#ifndef __cplusplus
+#error "This is a C++ header file."
+#endif
+
+#ifndef SVN_CXXHL_PRIVATE_EXCEPTION_HPP
+#define SVN_CXXHL_PRIVATE_EXCEPTION_HPP
+
+#include "svn_error.h"
+
+namespace apache {
+namespace subversion {
+namespace cxxhl {
+namespace detail {
+
+/**
+ * Given a @a err, if it is not @c NULL, convert it to a and throw an
+ * Error exception; otherwise do nothing.
+ */
+void checked_call(svn_error_t* err);
+
+} // namespace detail
+} // namespace cxxhl
+} // namespace subversion
+} // namespace apache
+
+#endif // SVN_CXXHL_PRIVATE_EXCEPTION_HPP
Propchange: subversion/trunk/subversion/bindings/cxxhl/src/private/exception.hpp
------------------------------------------------------------------------------
svn:eol-style = native
Modified: subversion/trunk/subversion/bindings/cxxhl/tests/test_exception.cpp
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/cxxhl/tests/test_exception.cpp?rev=1507413&r1=1507412&r2=1507413&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/cxxhl/tests/test_exception.cpp
(original)
+++ subversion/trunk/subversion/bindings/cxxhl/tests/test_exception.cpp Fri Jul
26 19:47:27 2013
@@ -28,6 +28,7 @@
#include <iostream>
#include "svncxxhl.hpp"
+#include "../src/cxxhl-private.hpp"
#include <apr.h>
#include "svn_error.h"
@@ -36,11 +37,11 @@ namespace {
void trace(const SVN::Error::Message& msg)
{
std::cout << " ";
- if (msg.first)
+ if (!msg.trace())
std::cout << "test_exception: E"
<< std::setw(6) << std::setfill('0') << std::right
- << msg.first << ':' << ' ';
- std::cout << msg.second << std::endl;
+ << msg.code() << ':' << ' ';
+ std::cout << msg.message() << std::endl;
}
void traceall(const char *message, const SVN::Error& err)
@@ -92,7 +93,7 @@ bool test_cancel()
{
try
{
- SVN::Error::throw_svn_error(make_cancel_test_error());
+ SVN::detail::checked_call(make_cancel_test_error());
}
catch (const SVN::Cancelled& err)
{
@@ -117,7 +118,7 @@ int test_error()
{
try
{
- SVN::Error::throw_svn_error(make_error_test_error());
+ SVN::detail::checked_call(make_error_test_error());
}
catch (const SVN::Cancelled& err)
{
