You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@wandisco.com> on 2010/03/18 15:30:23 UTC
Re: svn commit: r922867 - use of svn_error_compose()
On Wed, 2010-03-17, Greg Stein wrote:
> > + if (err && err->apr_err == SVN_ERR_WC_LOCKED)
> > + {
> > + svn_error_t *err2 = svn_wc__release_write_lock(wc_ctx,
> > + child_abspath,
> > + iterpool);
> > + if (err2)
> > + svn_error_compose(err, err2);
> > + return svn_error_return(err);
> > + }
> > + }
>
> Lately, we have not been using svn_error_compose(), favoring
> svn_error_compose_create() since it Does The Right Thing with the
> args, whether they're errors or not. Thus, the above code would
> typically be written:
>
> svn_error_t *err2 = svn_wc__release_write_lock(...);
>
> return svn_error_return(svn_err_compose_create(err, err2));
>
>
> Seeing the use of svn_error_compose() is a bit jarring :-P
Any reason not to extend svn_error_compose() to accept a NULL second
arg?
- Julian
Re: svn commit: r922867 - use of svn_error_compose()
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 18, 2010 at 11:30, Julian Foad <ju...@wandisco.com> wrote:
> On Wed, 2010-03-17, Greg Stein wrote:
>> > + if (err && err->apr_err == SVN_ERR_WC_LOCKED)
>> > + {
>> > + svn_error_t *err2 = svn_wc__release_write_lock(wc_ctx,
>> > + child_abspath,
>> > + iterpool);
>> > + if (err2)
>> > + svn_error_compose(err, err2);
>> > + return svn_error_return(err);
>> > + }
>> > + }
>>
>> Lately, we have not been using svn_error_compose(), favoring
>> svn_error_compose_create() since it Does The Right Thing with the
>> args, whether they're errors or not. Thus, the above code would
>> typically be written:
>>
>> svn_error_t *err2 = svn_wc__release_write_lock(...);
>>
>> return svn_error_return(svn_err_compose_create(err, err2));
>>
>>
>> Seeing the use of svn_error_compose() is a bit jarring :-P
>
> Any reason not to extend svn_error_compose() to accept a NULL second
> arg?
That would turn it into a noop, but that's fine.
IMO, we just shouldn't use it, but for a little extra safety it can
test for NULL.
Cheers,
-g