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, 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"?
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; > > > +} > > > > > >