Re: svn commit: r1849737 - in /subversion/trunk/subversion/bindings/cxx: include/svnxx.hpp include/svnxx/exception.hpp src/aprwrap/pool.hpp src/exception.cpp src/private/exception-private.hpp tests/test_exceptions.cpp

Branko Čibej wrote on Wed, 26 Dec 2018 22:41 +0100:
> On 26.12.2018 19:50, Daniel Shahaf wrote:
> > Haven't reviewed the rest of the patch, nor the mapping of
> > svn_error_t::apr_err values to this hierarchy.
>
> There is just one exception type that encapsulates all of svn_error_t,
> including the apr_err bit; that's 'svn::error'. I have no intention of
> going down the rabbit hole of having one exception type for each
> possible apr_err value!
>

Yeah, that'd be way too much; but I was thinking of two things:

1. apr_err can be an errno error code, not just an APR_OS_START_USERERR
   code. I don't have the C++ exceptions hierarchy in mind, but I
   suspect that when APR_STATUS_IS_EEXIST(err->apr_err), an svn::error
   instance is not what people (and 'catch' blocks) will expect.

2. SVN_ERROR_IN_CATEGORY(...). I'll say no more about it since we use
   it very rarely even on the C side of things.

> TL;DR:
>
> class error : public std::exception,
> protected std::shared_ptr<svn_error_t>
> {
> public:
> // ...
> const char* what(); // "best" error message (override)
> int code(); // converted apr_status_t
> const char* name(); // symbolic error name, when available
> // ...
> };

Thanks for this.

HTH,

Daniel
Received on 2018-12-26 23:29:23 CET