(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, danie...@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;
> > +}
> 
> 
> 

Reply via email to