On Wed, 2011-05-11, phi...@apache.org wrote:
> Author: philip
> Date: Wed May 11 17:24:46 2011
> New Revision: 1101990
>
> URL: http://svn.apache.org/viewvc?rev=1101990&view=rev
> Log:
> Avoid leaking errors about user input when some other runtime error occurs.
>
> * subversion/libsvn_client/cmdline.c
> (svn_client_args_to_target_array): Delay generating errors until returning.
Hi Philip. This changed the behaviour so it no longer matches its
documentation:
* If a path has the same name as a Subversion working copy
* administrative directory, return #SVN_ERR_RESERVED_FILENAME_SPECIFIED;
* if multiple reserved paths are encountered, return a chain of
* errors, all of which are #SVN_ERR_RESERVED_FILENAME_SPECIFIED. Do
* not return this type of error in a chain with any other type of
* error, and if this is the only type of error encountered, complete
* the operation before returning the error(s).
It should only generate and return error objects corresponding to the
reserved names if there is no other error. I think this patch should
fix it:
[[[
* subversion/libsvn_client/cmdline.c
(svn_client_args_to_target_array2): Restore documented error behaviour. A
follow-up to r1101990.
--This line, and those below, will be ignored--
Index: subversion/libsvn_client/cmdline.c
===================================================================
--- subversion/libsvn_client/cmdline.c (revision 1140096)
+++ subversion/libsvn_client/cmdline.c (working copy)
@@ -347,20 +347,10 @@ svn_client_args_to_target_array2(apr_arr
{
err = svn_client_root_url_from_path(&root_url, "", ctx, pool);
if (err || root_url == NULL)
- {
- if (reserved_names)
- for (i = 0; i < reserved_names->nelts; ++i)
- err = svn_error_createf(SVN_ERR_RESERVED_FILENAME_SPECIFIED,
- err,
- _("'%s' ends in a reserved name"),
- APR_ARRAY_IDX(reserved_names, i,
- const char *));
-
- return svn_error_create(SVN_ERR_WC_NOT_WORKING_COPY, err,
- _("Resolving '^/': no repository root "
- "found in the target arguments or "
- "in the current directory"));
- }
+ return svn_error_create(SVN_ERR_WC_NOT_WORKING_COPY, err,
+ _("Resolving '^/': no repository root "
+ "found in the target arguments or "
+ "in the current directory"));
}
*targets_p = apr_array_make(pool, output_targets->nelts,
@@ -395,7 +385,7 @@ svn_client_args_to_target_array2(apr_arr
else
*targets_p = output_targets;
- if (reserved_names)
+ if (reserved_names && ! err)
for (i = 0; i < reserved_names->nelts; ++i)
err = svn_error_createf(SVN_ERR_RESERVED_FILENAME_SPECIFIED, err,
_("'%s' ends in a reserved name"),
]]]
Agree?
- Julian
> Modified: subversion/trunk/subversion/libsvn_client/cmdline.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/cmdline.c?rev=1101990&r1=1101989&r2=1101990&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/cmdline.c (original)
> +++ subversion/trunk/subversion/libsvn_client/cmdline.c Wed May 11 17:24:46
> 2011
> @@ -172,6 +172,7 @@ svn_client_args_to_target_array(apr_arra
> apr_array_make(pool, DEFAULT_ARRAY_SIZE, sizeof(const char *));
> apr_array_header_t *output_targets =
> apr_array_make(pool, DEFAULT_ARRAY_SIZE, sizeof(const char *));
> + apr_array_header_t *reserved_names = NULL;
>
> /* Step 1: create a master array of targets that are in UTF-8
> encoding, and come from concatenating the targets left by apr_getopt,
> @@ -263,12 +264,12 @@ svn_client_args_to_target_array(apr_arra
>
> if (svn_wc_is_adm_dir(base_name, pool))
> {
> - err =
> svn_error_createf(SVN_ERR_RESERVED_FILENAME_SPECIFIED,
> - err,
> - _("'%s' ends in a reserved name"),
> - utf8_target);
> + if (!reserved_names)
> + reserved_names = apr_array_make(pool, DEFAULT_ARRAY_SIZE,
> + sizeof(const char *));
> +
> + APR_ARRAY_PUSH(reserved_names, const char *) = utf8_target;
>
> - /* ### Error leaks! Mixing SVN_ERR with err is a leak */
> continue;
> }
> }
> @@ -296,14 +297,22 @@ svn_client_args_to_target_array(apr_arra
> */
> if (root_url == NULL)
> {
> - svn_error_t *err2;
> - err2 = svn_client_root_url_from_path(&root_url, "", ctx, pool);
> + err = svn_client_root_url_from_path(&root_url, "", ctx, pool);
> + if (err || root_url == NULL)
> + {
> + if (reserved_names)
> + for (i = 0; i < reserved_names->nelts; ++i)
> + err =
> svn_error_createf(SVN_ERR_RESERVED_FILENAME_SPECIFIED,
> + err,
> + _("'%s' ends in a reserved name"),
> + APR_ARRAY_IDX(reserved_names, i,
> + const char *));
>
> - if (err2 || root_url == NULL)
> - return svn_error_create(SVN_ERR_WC_NOT_WORKING_COPY, err2,
> - _("Resolving '^/': no repository root "
> - "found in the target arguments or "
> - "in the current directory"));
> + return svn_error_create(SVN_ERR_WC_NOT_WORKING_COPY, err,
> + _("Resolving '^/': no repository root "
> + "found in the target arguments or "
> + "in the current directory"));
> + }
> }
>
> *targets_p = apr_array_make(pool, output_targets->nelts,
> @@ -338,5 +347,11 @@ svn_client_args_to_target_array(apr_arra
> else
> *targets_p = output_targets;
>
> + if (reserved_names)
> + for (i = 0; i < reserved_names->nelts; ++i)
> + err = svn_error_createf(SVN_ERR_RESERVED_FILENAME_SPECIFIED, err,
> + _("'%s' ends in a reserved name"),
> + APR_ARRAY_IDX(reserved_names, i, const char
> *));
> +
> return svn_error_return(err);
> }
>
>
