You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2010/08/09 22:29:48 UTC
svn commit: r983807 - in /subversion/branches/atomic-revprop/subversion:
include/svn_error.h libsvn_subr/error.c
Author: danielsh
Date: Mon Aug 9 20:29:48 2010
New Revision: 983807
URL: http://svn.apache.org/viewvc?rev=983807&view=rev
Log:
Add a helper error API.
* subversion/include/svn_error.h
(svn_error_has_cause): New declaration.
* subversion/libsvn_subr/error.c
(svn_error_has_cause): New definition.
Modified:
subversion/branches/atomic-revprop/subversion/include/svn_error.h
subversion/branches/atomic-revprop/subversion/libsvn_subr/error.c
Modified: subversion/branches/atomic-revprop/subversion/include/svn_error.h
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/include/svn_error.h?rev=983807&r1=983806&r2=983807&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/include/svn_error.h (original)
+++ subversion/branches/atomic-revprop/subversion/include/svn_error.h Mon Aug 9 20:29:48 2010
@@ -193,6 +193,13 @@ svn_error_compose(svn_error_t *chain,
svn_error_t *
svn_error_root_cause(svn_error_t *err);
+/** Return TRUE if @a err's chain contains the error code @a apr_err.
+ *
+ * @since New in 1.7.
+ */
+svn_boolean_t
+svn_error_has_cause(svn_error_t *err, apr_status_t apr_err);
+
/** Create a new error that is a deep copy of @a err and return it.
*
* @since New in 1.2.
Modified: subversion/branches/atomic-revprop/subversion/libsvn_subr/error.c
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_subr/error.c?rev=983807&r1=983806&r2=983807&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/libsvn_subr/error.c (original)
+++ subversion/branches/atomic-revprop/subversion/libsvn_subr/error.c Mon Aug 9 20:29:48 2010
@@ -269,6 +269,22 @@ svn_error_root_cause(svn_error_t *err)
return err;
}
+svn_boolean_t
+svn_error_has_cause(svn_error_t *err, apr_status_t apr_err)
+{
+ svn_error_t *child;
+
+ if (! err && ! apr_err)
+ /* The API doesn't specify the behaviour when ERR is NULL. */
+ return TRUE;
+
+ for (child = err; child; child = child->child)
+ if (child->apr_err == apr_err)
+ return TRUE;
+
+ return FALSE;
+}
+
svn_error_t *
svn_error_dup(svn_error_t *err)
{
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Tue, Aug 10, 2010 at 14:33:24 +0100:
> On Tue, 2010-08-10, Daniel Shahaf wrote:
> > +++ subversion/libsvn_subr/error.c (working copy)
> > @@ -274,9 +274,8 @@
> > {
> > svn_error_t *child;
> >
> > - if (! err && ! apr_err)
> > - /* The API doesn't specify the behaviour when ERR is NULL. */
> > - return TRUE;
> > + if (err == SVN_NO_ERROR)
> > + return FALSE;
>
> No need to implement this check as a special case: the general case loop
> below already does the same.
Okay. Committed in r984264.
Thanks,
Daniel
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Tue, Aug 10, 2010 at 14:33:24 +0100:
> On Tue, 2010-08-10, Daniel Shahaf wrote:
> > +++ subversion/libsvn_subr/error.c (working copy)
> > @@ -274,9 +274,8 @@
> > {
> > svn_error_t *child;
> >
> > - if (! err && ! apr_err)
> > - /* The API doesn't specify the behaviour when ERR is NULL. */
> > - return TRUE;
> > + if (err == SVN_NO_ERROR)
> > + return FALSE;
>
> No need to implement this check as a special case: the general case loop
> below already does the same.
Okay. Committed in r984264.
Thanks,
Daniel
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-08-10, Daniel Shahaf wrote:
> Julian Foad wrote on Tue, Aug 10, 2010 at 13:48:20 +0100:
> > On Tue, 2010-08-10 at 15:04 +0300, Daniel Shahaf wrote:
> > > I can see several options:
> > >
> > > * forbid passing SVN_NO_ERROR
> > > * return FALSE on SVN_NO_ERROR
> > > * reture (apr_err == APR_SUCCESS ? TRUE : FALSE) on SVN_NO_ERROR
> > >
> > > Right now, the API does the first and the code the third.
> > >
> > >
> > > I suppose the question is (1) whether we expect the caller to test for
> > > SVN_NO_ERROR before calling this function, and (2) whether we expect people to
> > > pass APR_SUCCESS to this function. Personally my answers (while writing
> > > this) were: (1) yes (2) no.
> >
> > Generally we have found that with error testing it's convenient to be
> > able to write "err = foo(); do_something_with(err);" without having to
> > check "err" is non-null first.
>
> Yep, and I just realized that one of my pending patches also missed the "if
> (err)" check. How about:
>
> [[[
> Index: subversion/libsvn_subr/error.c
> ===================================================================
> --- subversion/libsvn_subr/error.c (revision 983929)
> +++ subversion/libsvn_subr/error.c (working copy)
> @@ -274,9 +274,8 @@
> {
> svn_error_t *child;
>
> - if (! err && ! apr_err)
> - /* The API doesn't specify the behaviour when ERR is NULL. */
> - return TRUE;
> + if (err == SVN_NO_ERROR)
> + return FALSE;
No need to implement this check as a special case: the general case loop
below already does the same.
> for (child = err; child; child = child->child)
> if (child->apr_err == apr_err)
> Index: subversion/include/svn_error.h
> ===================================================================
> --- subversion/include/svn_error.h (revision 983929)
> +++ subversion/include/svn_error.h (working copy)
> @@ -195,6 +195,8 @@
>
> /** Return TRUE if @a err's chain contains the error code @a apr_err.
> *
> + * If @a err is #SVN_NO_ERROR, return FALSE.
> + *
> * @since New in 1.7.
> */
> svn_boolean_t
> ]]]
>
> > I agree with (2) - I don't think it makes much sense to pass APR_SUCCESS
> > to this function.
>
> Of course, but I don't think we should bother to special-case/check
> for that.
Agreed.
- Julian
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-08-10, Daniel Shahaf wrote:
> Julian Foad wrote on Tue, Aug 10, 2010 at 13:48:20 +0100:
> > On Tue, 2010-08-10 at 15:04 +0300, Daniel Shahaf wrote:
> > > I can see several options:
> > >
> > > * forbid passing SVN_NO_ERROR
> > > * return FALSE on SVN_NO_ERROR
> > > * reture (apr_err == APR_SUCCESS ? TRUE : FALSE) on SVN_NO_ERROR
> > >
> > > Right now, the API does the first and the code the third.
> > >
> > >
> > > I suppose the question is (1) whether we expect the caller to test for
> > > SVN_NO_ERROR before calling this function, and (2) whether we expect people to
> > > pass APR_SUCCESS to this function. Personally my answers (while writing
> > > this) were: (1) yes (2) no.
> >
> > Generally we have found that with error testing it's convenient to be
> > able to write "err = foo(); do_something_with(err);" without having to
> > check "err" is non-null first.
>
> Yep, and I just realized that one of my pending patches also missed the "if
> (err)" check. How about:
>
> [[[
> Index: subversion/libsvn_subr/error.c
> ===================================================================
> --- subversion/libsvn_subr/error.c (revision 983929)
> +++ subversion/libsvn_subr/error.c (working copy)
> @@ -274,9 +274,8 @@
> {
> svn_error_t *child;
>
> - if (! err && ! apr_err)
> - /* The API doesn't specify the behaviour when ERR is NULL. */
> - return TRUE;
> + if (err == SVN_NO_ERROR)
> + return FALSE;
No need to implement this check as a special case: the general case loop
below already does the same.
> for (child = err; child; child = child->child)
> if (child->apr_err == apr_err)
> Index: subversion/include/svn_error.h
> ===================================================================
> --- subversion/include/svn_error.h (revision 983929)
> +++ subversion/include/svn_error.h (working copy)
> @@ -195,6 +195,8 @@
>
> /** Return TRUE if @a err's chain contains the error code @a apr_err.
> *
> + * If @a err is #SVN_NO_ERROR, return FALSE.
> + *
> * @since New in 1.7.
> */
> svn_boolean_t
> ]]]
>
> > I agree with (2) - I don't think it makes much sense to pass APR_SUCCESS
> > to this function.
>
> Of course, but I don't think we should bother to special-case/check
> for that.
Agreed.
- Julian
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Tue, Aug 10, 2010 at 13:48:20 +0100:
> On Tue, 2010-08-10 at 15:04 +0300, Daniel Shahaf wrote:
> > I can see several options:
> >
> > * forbid passing SVN_NO_ERROR
> > * return FALSE on SVN_NO_ERROR
> > * reture (apr_err == APR_SUCCESS ? TRUE : FALSE) on SVN_NO_ERROR
> >
> > Right now, the API does the first and the code the third.
> >
> >
> > I suppose the question is (1) whether we expect the caller to test for
> > SVN_NO_ERROR before calling this function, and (2) whether we expect people to
> > pass APR_SUCCESS to this function. Personally my answers (while writing
> > this) were: (1) yes (2) no.
>
> Generally we have found that with error testing it's convenient to be
> able to write "err = foo(); do_something_with(err);" without having to
> check "err" is non-null first.
Yep, and I just realized that one of my pending patches also missed the "if
(err)" check. How about:
[[[
Index: subversion/libsvn_subr/error.c
===================================================================
--- subversion/libsvn_subr/error.c (revision 983929)
+++ subversion/libsvn_subr/error.c (working copy)
@@ -274,9 +274,8 @@
{
svn_error_t *child;
- if (! err && ! apr_err)
- /* The API doesn't specify the behaviour when ERR is NULL. */
- return TRUE;
+ if (err == SVN_NO_ERROR)
+ return FALSE;
for (child = err; child; child = child->child)
if (child->apr_err == apr_err)
Index: subversion/include/svn_error.h
===================================================================
--- subversion/include/svn_error.h (revision 983929)
+++ subversion/include/svn_error.h (working copy)
@@ -195,6 +195,8 @@
/** Return TRUE if @a err's chain contains the error code @a apr_err.
*
+ * If @a err is #SVN_NO_ERROR, return FALSE.
+ *
* @since New in 1.7.
*/
svn_boolean_t
]]]
> I agree with (2) - I don't think it makes much sense to pass APR_SUCCESS
> to this function.
Of course, but I don't think we should bother to special-case/check
for that.
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Tue, Aug 10, 2010 at 13:48:20 +0100:
> On Tue, 2010-08-10 at 15:04 +0300, Daniel Shahaf wrote:
> > I can see several options:
> >
> > * forbid passing SVN_NO_ERROR
> > * return FALSE on SVN_NO_ERROR
> > * reture (apr_err == APR_SUCCESS ? TRUE : FALSE) on SVN_NO_ERROR
> >
> > Right now, the API does the first and the code the third.
> >
> >
> > I suppose the question is (1) whether we expect the caller to test for
> > SVN_NO_ERROR before calling this function, and (2) whether we expect people to
> > pass APR_SUCCESS to this function. Personally my answers (while writing
> > this) were: (1) yes (2) no.
>
> Generally we have found that with error testing it's convenient to be
> able to write "err = foo(); do_something_with(err);" without having to
> check "err" is non-null first.
Yep, and I just realized that one of my pending patches also missed the "if
(err)" check. How about:
[[[
Index: subversion/libsvn_subr/error.c
===================================================================
--- subversion/libsvn_subr/error.c (revision 983929)
+++ subversion/libsvn_subr/error.c (working copy)
@@ -274,9 +274,8 @@
{
svn_error_t *child;
- if (! err && ! apr_err)
- /* The API doesn't specify the behaviour when ERR is NULL. */
- return TRUE;
+ if (err == SVN_NO_ERROR)
+ return FALSE;
for (child = err; child; child = child->child)
if (child->apr_err == apr_err)
Index: subversion/include/svn_error.h
===================================================================
--- subversion/include/svn_error.h (revision 983929)
+++ subversion/include/svn_error.h (working copy)
@@ -195,6 +195,8 @@
/** Return TRUE if @a err's chain contains the error code @a apr_err.
*
+ * If @a err is #SVN_NO_ERROR, return FALSE.
+ *
* @since New in 1.7.
*/
svn_boolean_t
]]]
> I agree with (2) - I don't think it makes much sense to pass APR_SUCCESS
> to this function.
Of course, but I don't think we should bother to special-case/check
for that.
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-08-10 at 15:04 +0300, Daniel Shahaf wrote:
> (I intended to commit that to trunk)
>
> Julian Foad wrote on Tue, Aug 10, 2010 at 12:17:24 +0100:
> > On Mon, 2010-08-09, danielsh@apache.org wrote:
> > >
> > > +/** Return TRUE if @a err's chain contains the error code @a apr_err.
> > > + *
> > > + * @since New in 1.7.
> > > + */
> > > +svn_boolean_t
> > > +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err);
> >
> > This looks like it could be a useful API.
> >
>
> I'll need it to dig beneath ra_svn's wrapping of server-generated errors by
> another error code.
>
> > I would expect to be able to call such a function on a NULL error
> > pointer, and the result should be FALSE since "no error" doesn't have
> > any cause that can be expressed in an apr_status_t.
>
> Ahem, doesn't (apr_status_t)APR_SUCCESS mean "no error"?
Yes but it's not really "a cause of the error".
> >
> > > +svn_boolean_t
> > > +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err)
> > > +{
> > > + svn_error_t *child;
> > > +
> > > + if (! err && ! apr_err)
> > > + /* The API doesn't specify the behaviour when ERR is NULL. */
> > > + return TRUE;
> >
> > What's this block for?
>
> To make svn_error_has_cause(SVN_NO_ERROR, apr_err) return TRUE if and only
> if apr_err == APR_SUCCESS.
>
> > The behaviour I mentioned above would fall out
> > from just removing this block. It looks like this is so that you can
> > write svn_error_has_cause(err, APR_SUCCESS) and get TRUE if ERR is NULL
> > or contains APR_SUCCESS anywhere in its chain, but is that really
> > useful?
> >
>
> I didn't consider the option that APR_SUCCESS could occur as part of the
> chain. Good point.
>
> > - Julian
> >
>
> I can see several options:
>
> * forbid passing SVN_NO_ERROR
> * return FALSE on SVN_NO_ERROR
> * reture (apr_err == APR_SUCCESS ? TRUE : FALSE) on SVN_NO_ERROR
>
> Right now, the API does the first and the code the third.
>
>
> I suppose the question is (1) whether we expect the caller to test for
> SVN_NO_ERROR before calling this function, and (2) whether we expect people to
> pass APR_SUCCESS to this function. Personally my answers (while writing
> this) were: (1) yes (2) no.
Generally we have found that with error testing it's convenient to be
able to write "err = foo(); do_something_with(err);" without having to
check "err" is non-null first. I agree with (2) - I don't think it
makes much sense to pass APR_SUCCESS to this function.
- Julian
> >
> > > + for (child = err; child; child = child->child)
> > > + if (child->apr_err == apr_err)
> > > + return TRUE;
> > > +
> > > + return FALSE;
> > > +}
> >
> >
> >
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-08-10 at 15:04 +0300, Daniel Shahaf wrote:
> (I intended to commit that to trunk)
>
> Julian Foad wrote on Tue, Aug 10, 2010 at 12:17:24 +0100:
> > On Mon, 2010-08-09, danielsh@apache.org wrote:
> > >
> > > +/** Return TRUE if @a err's chain contains the error code @a apr_err.
> > > + *
> > > + * @since New in 1.7.
> > > + */
> > > +svn_boolean_t
> > > +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err);
> >
> > This looks like it could be a useful API.
> >
>
> I'll need it to dig beneath ra_svn's wrapping of server-generated errors by
> another error code.
>
> > I would expect to be able to call such a function on a NULL error
> > pointer, and the result should be FALSE since "no error" doesn't have
> > any cause that can be expressed in an apr_status_t.
>
> Ahem, doesn't (apr_status_t)APR_SUCCESS mean "no error"?
Yes but it's not really "a cause of the error".
> >
> > > +svn_boolean_t
> > > +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err)
> > > +{
> > > + svn_error_t *child;
> > > +
> > > + if (! err && ! apr_err)
> > > + /* The API doesn't specify the behaviour when ERR is NULL. */
> > > + return TRUE;
> >
> > What's this block for?
>
> To make svn_error_has_cause(SVN_NO_ERROR, apr_err) return TRUE if and only
> if apr_err == APR_SUCCESS.
>
> > The behaviour I mentioned above would fall out
> > from just removing this block. It looks like this is so that you can
> > write svn_error_has_cause(err, APR_SUCCESS) and get TRUE if ERR is NULL
> > or contains APR_SUCCESS anywhere in its chain, but is that really
> > useful?
> >
>
> I didn't consider the option that APR_SUCCESS could occur as part of the
> chain. Good point.
>
> > - Julian
> >
>
> I can see several options:
>
> * forbid passing SVN_NO_ERROR
> * return FALSE on SVN_NO_ERROR
> * reture (apr_err == APR_SUCCESS ? TRUE : FALSE) on SVN_NO_ERROR
>
> Right now, the API does the first and the code the third.
>
>
> I suppose the question is (1) whether we expect the caller to test for
> SVN_NO_ERROR before calling this function, and (2) whether we expect people to
> pass APR_SUCCESS to this function. Personally my answers (while writing
> this) were: (1) yes (2) no.
Generally we have found that with error testing it's convenient to be
able to write "err = foo(); do_something_with(err);" without having to
check "err" is non-null first. I agree with (2) - I don't think it
makes much sense to pass APR_SUCCESS to this function.
- Julian
> >
> > > + for (child = err; child; child = child->child)
> > > + if (child->apr_err == apr_err)
> > > + return TRUE;
> > > +
> > > + return FALSE;
> > > +}
> >
> >
> >
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
(I intended to commit that to trunk)
Julian Foad wrote on Tue, Aug 10, 2010 at 12:17:24 +0100:
> On Mon, 2010-08-09, danielsh@apache.org wrote:
> >
> > +/** Return TRUE if @a err's chain contains the error code @a apr_err.
> > + *
> > + * @since New in 1.7.
> > + */
> > +svn_boolean_t
> > +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err);
>
> This looks like it could be a useful API.
>
I'll need it to dig beneath ra_svn's wrapping of server-generated errors by
another error code.
> I would expect to be able to call such a function on a NULL error
> pointer, and the result should be FALSE since "no error" doesn't have
> any cause that can be expressed in an apr_status_t.
Ahem, doesn't (apr_status_t)APR_SUCCESS mean "no error"?
>
> > +svn_boolean_t
> > +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err)
> > +{
> > + svn_error_t *child;
> > +
> > + if (! err && ! apr_err)
> > + /* The API doesn't specify the behaviour when ERR is NULL. */
> > + return TRUE;
>
> What's this block for?
To make svn_error_has_cause(SVN_NO_ERROR, apr_err) return TRUE if and only
if apr_err == APR_SUCCESS.
> The behaviour I mentioned above would fall out
> from just removing this block. It looks like this is so that you can
> write svn_error_has_cause(err, APR_SUCCESS) and get TRUE if ERR is NULL
> or contains APR_SUCCESS anywhere in its chain, but is that really
> useful?
>
I didn't consider the option that APR_SUCCESS could occur as part of the
chain. Good point.
> - Julian
>
I can see several options:
* forbid passing SVN_NO_ERROR
* return FALSE on SVN_NO_ERROR
* reture (apr_err == APR_SUCCESS ? TRUE : FALSE) on SVN_NO_ERROR
Right now, the API does the first and the code the third.
I suppose the question is (1) whether we expect the caller to test for
SVN_NO_ERROR before calling this function, and (2) whether we expect people to
pass APR_SUCCESS to this function. Personally my answers (while writing
this) were: (1) yes (2) no.
>
> > + for (child = err; child; child = child->child)
> > + if (child->apr_err == apr_err)
> > + return TRUE;
> > +
> > + return FALSE;
> > +}
>
>
>
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
(I intended to commit that to trunk)
Julian Foad wrote on Tue, Aug 10, 2010 at 12:17:24 +0100:
> On Mon, 2010-08-09, danielsh@apache.org wrote:
> >
> > +/** Return TRUE if @a err's chain contains the error code @a apr_err.
> > + *
> > + * @since New in 1.7.
> > + */
> > +svn_boolean_t
> > +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err);
>
> This looks like it could be a useful API.
>
I'll need it to dig beneath ra_svn's wrapping of server-generated errors by
another error code.
> I would expect to be able to call such a function on a NULL error
> pointer, and the result should be FALSE since "no error" doesn't have
> any cause that can be expressed in an apr_status_t.
Ahem, doesn't (apr_status_t)APR_SUCCESS mean "no error"?
>
> > +svn_boolean_t
> > +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err)
> > +{
> > + svn_error_t *child;
> > +
> > + if (! err && ! apr_err)
> > + /* The API doesn't specify the behaviour when ERR is NULL. */
> > + return TRUE;
>
> What's this block for?
To make svn_error_has_cause(SVN_NO_ERROR, apr_err) return TRUE if and only
if apr_err == APR_SUCCESS.
> The behaviour I mentioned above would fall out
> from just removing this block. It looks like this is so that you can
> write svn_error_has_cause(err, APR_SUCCESS) and get TRUE if ERR is NULL
> or contains APR_SUCCESS anywhere in its chain, but is that really
> useful?
>
I didn't consider the option that APR_SUCCESS could occur as part of the
chain. Good point.
> - Julian
>
I can see several options:
* forbid passing SVN_NO_ERROR
* return FALSE on SVN_NO_ERROR
* reture (apr_err == APR_SUCCESS ? TRUE : FALSE) on SVN_NO_ERROR
Right now, the API does the first and the code the third.
I suppose the question is (1) whether we expect the caller to test for
SVN_NO_ERROR before calling this function, and (2) whether we expect people to
pass APR_SUCCESS to this function. Personally my answers (while writing
this) were: (1) yes (2) no.
>
> > + for (child = err; child; child = child->child)
> > + if (child->apr_err == apr_err)
> > + return TRUE;
> > +
> > + return FALSE;
> > +}
>
>
>
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2010-08-09, danielsh@apache.org wrote:
>
> +/** Return TRUE if @a err's chain contains the error code @a apr_err.
> + *
> + * @since New in 1.7.
> + */
> +svn_boolean_t
> +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err);
This looks like it could be a useful API.
I would expect to be able to call such a function on a NULL error
pointer, and the result should be FALSE since "no error" doesn't have
any cause that can be expressed in an apr_status_t.
> +svn_boolean_t
> +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err)
> +{
> + svn_error_t *child;
> +
> + if (! err && ! apr_err)
> + /* The API doesn't specify the behaviour when ERR is NULL. */
> + return TRUE;
What's this block for? The behaviour I mentioned above would fall out
from just removing this block. It looks like this is so that you can
write svn_error_has_cause(err, APR_SUCCESS) and get TRUE if ERR is NULL
or contains APR_SUCCESS anywhere in its chain, but is that really
useful?
- Julian
> + for (child = err; child; child = child->child)
> + if (child->apr_err == apr_err)
> + return TRUE;
> +
> + return FALSE;
> +}
Re: svn commit: r983807 - in
/subversion/branches/atomic-revprop/subversion: include/svn_error.h
libsvn_subr/error.c
Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2010-08-09, danielsh@apache.org wrote:
>
> +/** Return TRUE if @a err's chain contains the error code @a apr_err.
> + *
> + * @since New in 1.7.
> + */
> +svn_boolean_t
> +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err);
This looks like it could be a useful API.
I would expect to be able to call such a function on a NULL error
pointer, and the result should be FALSE since "no error" doesn't have
any cause that can be expressed in an apr_status_t.
> +svn_boolean_t
> +svn_error_has_cause(svn_error_t *err, apr_status_t apr_err)
> +{
> + svn_error_t *child;
> +
> + if (! err && ! apr_err)
> + /* The API doesn't specify the behaviour when ERR is NULL. */
> + return TRUE;
What's this block for? The behaviour I mentioned above would fall out
from just removing this block. It looks like this is so that you can
write svn_error_has_cause(err, APR_SUCCESS) and get TRUE if ERR is NULL
or contains APR_SUCCESS anywhere in its chain, but is that really
useful?
- Julian
> + for (child = err; child; child = child->child)
> + if (child->apr_err == apr_err)
> + return TRUE;
> +
> + return FALSE;
> +}