Re: svn commit: r29136 - trunk/subversion/libsvn_subr

David Glasser wrote:
> On Feb 1, 2008 9:31 AM, <hwright_at_tigris.org> wrote:
>> Author: hwright
>> Date: Fri Feb 1 09:31:43 2008
>> New Revision: 29136
>>
>> Log:
>> Followup to r29127: Cleanup error handling.
>>
>> * subversion/libsvn_subr/opt.c
>> (svn_opt_args_to_target_array2): Rename error variable, simplify error
>> handling logic.
>> (svn_opt_args_to_target_array3): Rename error variable, simplify error
>> creation and usage.
>>
>>
>> Modified:
>> trunk/subversion/libsvn_subr/opt.c
>>
>> Modified: trunk/subversion/libsvn_subr/opt.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/opt.c?pathrev=29136&r1=29135&r2=29136
>> ==============================================================================
>> --- trunk/subversion/libsvn_subr/opt.c (original)
>> +++ trunk/subversion/libsvn_subr/opt.c Fri Feb 1 09:31:43 2008
>> @@ -872,19 +872,18 @@
>> apr_array_header_t *known_targets,
>> apr_pool_t *pool)
>> {
>> - svn_error_t *error = svn_opt_args_to_target_array3(targets_p,
>> + svn_error_t *err = svn_opt_args_to_target_array3(targets_p,
>> os,
>> known_targets,
>> pool);
>> - if (error)
>> +
>> + if (err && err->apr_err == SVN_ERR_RESERVED_FILENAME_SPECIFIED)
>> {
>> - if (error->apr_err == SVN_ERR_RESERVED_FILENAME_SPECIFIED)
>> - svn_error_clear(error);
>> - else
>> - return error;
>> + svn_error_clear(err);
>> + return SVN_NO_ERROR;
>> }
>> - return SVN_NO_ERROR;
>> -
>> +
>> + return err;
>> }
>>
>>
>> @@ -895,7 +894,7 @@
>> apr_pool_t *pool)
>> {
>> int i;
>> - svn_error_t *error = SVN_NO_ERROR;
>> + svn_error_t *err = SVN_NO_ERROR;
>> apr_array_header_t *input_targets =
>> apr_array_make(pool, DEFAULT_ARRAY_SIZE, sizeof(const char *));
>> apr_array_header_t *output_targets =
>> @@ -1034,11 +1033,9 @@
>> if (0 == strcmp(base_name, ".svn")
>> || 0 == strcmp(base_name, "_svn"))
>> {
>> - char *error_str = apr_psprintf(pool, _("'%s' ends in a "
>> - "reserved name"), target);
>> - error = svn_error_create(SVN_ERR_RESERVED_FILENAME_SPECIFIED,
>> - error,
>> - error_str);
>> + err = svn_error_createf(SVN_ERR_RESERVED_FILENAME_SPECIFIED,
>> + NULL, _("'%s' ends in a reserved name"),
>> + target);
>
> Whoa there. You changed this from chaining the errors to not chaining
> the errors. That's both an error leak, and a loss of information.
> Did you mean to do that?

whoops! I *didn't* mean to do that. Thanks for the catch! (Fixed,
hopefully, in r29138.)

> --dave, who after the previous revision, actually had to go in and
> read svn_handle_error2 and make sure that error chaining worked the
> way it needed to to make it work...