David Glasser wrote:
> On Jan 31, 2008 8:15 PM, <kfogel_at_tigris.org> wrote:
>> Author: kfogel
>> Date: Thu Jan 31 20:15:40 2008
>> New Revision: 29127
>>
>> Log:
>> Fix issue #3026: _svn and .svn are silently ignored as file arguments.
>>
>> Patch by: Augie Fackler: <durin42_at_gmail.com>
>> (Test by me.)
>> Modified: trunk/subversion/include/svn_error_codes.h
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_error_codes.h?pathrev=29127&r1=29126&r2=29127
>> ==============================================================================
>> --- trunk/subversion/include/svn_error_codes.h (original)
>> +++ trunk/subversion/include/svn_error_codes.h Thu Jan 31 20:15:40 2008
>> @@ -1124,6 +1124,10 @@
>> SVN_ERRDEF(SVN_ERR_UNKNOWN_CHANGELIST,
>> SVN_ERR_MISC_CATEGORY_START + 24,
>> "Unknown changelist")
>> +
>> + SVN_ERRDEF(SVN_ERR_RESERVED_FILENAME_SPECIFIED,
>> + SVN_ERR_MISC_CATEGORY_START + 25,
>> + "Reserved directory name in command line arguments")
>
> Indentation is weird.
>
>> /* command-line client errors */
>>
>>
>> Modified: trunk/subversion/include/svn_opt.h
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_opt.h?pathrev=29127&r1=29126&r2=29127
>> ==============================================================================
>> --- trunk/subversion/include/svn_opt.h (original)
>> +++ trunk/subversion/include/svn_opt.h Thu Jan 31 20:15:40 2008
>> @@ -490,6 +490,18 @@
>> apr_array_header_t *known_targets,
>> apr_pool_t *pool);
>>
>> +/**
>> + * This is the same as svn_opt_args_to_target_array2() except that it returns
>> + * errors when a path has the same name as a working copy directory.
>> + *
>> + * @since New in 1.5.
>> + */
>
> We should always document the newest version, and change the old
> version to say how it differs. I also find this to be confusing: I'd
> say "a working copy administrative directory". Also, it should be
> explicit about the fact that while it returns an error (and probably
> should say which one), it still actually completes the operation.
> (Please do not backport this revision without fixing this, and the
> related issue mentioned below!)
>
>> +svn_error_t *
>> +svn_opt_args_to_target_array3(apr_array_header_t **targets_p,
>> + apr_getopt_t *os,
>> + apr_array_header_t *known_targets,
>> + apr_pool_t *pool);
>> +
>>
>> /**
>> * The same as svn_opt_args_to_target_array2() except that, in
>>
>> Modified: trunk/subversion/libsvn_subr/opt.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/opt.c?pathrev=29127&r1=29126&r2=29127
>> ==============================================================================
>> --- trunk/subversion/libsvn_subr/opt.c (original)
>> +++ trunk/subversion/libsvn_subr/opt.c Thu Jan 31 20:15:40 2008
>> @@ -872,7 +872,30 @@
>> apr_array_header_t *known_targets,
>> apr_pool_t *pool)
>> {
>> + svn_error_t *error = svn_opt_args_to_target_array3(targets_p,
>> + os,
>> + known_targets,
>> + pool);
>> + if (error)
>> + {
>> + if (error->apr_err == SVN_ERR_RESERVED_FILENAME_SPECIFIED)
>> + svn_error_clear(error);
>> + else
>> + return error;
>> + }
>> + return SVN_NO_ERROR;
>> +
>> +}
>> +
>> +
>> +svn_error_t *
>> +svn_opt_args_to_target_array3(apr_array_header_t **targets_p,
>> + apr_getopt_t *os,
>> + apr_array_header_t *known_targets,
>> + apr_pool_t *pool)
>> +{
>> int i;
>> + svn_error_t *error = 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 =
>> @@ -1010,7 +1033,14 @@
>> synchronized! */
>> if (0 == strcmp(base_name, ".svn")
>> || 0 == strcmp(base_name, "_svn"))
>> - continue;
>> + {
>> + 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);
>> + continue;
>
> Hmm, why not use svn_error_createf?
Fixed in r29136.
>> + }
>> }
>>
>> /* Append the peg revision back to the canonicalized target if
>> @@ -1026,6 +1056,8 @@
>> passing it to the cmd_func. */
>>
>> *targets_p = output_targets;
>> + if (error)
>> + return error;
>> return SVN_NO_ERROR;
>> }
>>
>>
>
>> Modified: trunk/subversion/svn/util.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/util.c?pathrev=29127&r1=29126&r2=29127
>> ==============================================================================
>> --- trunk/subversion/svn/util.c (original)
>> +++ trunk/subversion/svn/util.c Thu Jan 31 20:15:40 2008
>> @@ -1010,6 +1010,28 @@
>> }
>>
>>
>> +svn_error_t *
>> +svn_cl__args_to_target_array_print_reserved(apr_array_header_t **targets,
>> + apr_getopt_t *os,
>> + apr_array_header_t *known_targets,
>> + apr_pool_t *pool)
>> +{
>> + svn_error_t *error = svn_opt_args_to_target_array3(targets, os,
>> + known_targets, pool);
>> + if (error)
>> + {
>> + if (error->apr_err == SVN_ERR_RESERVED_FILENAME_SPECIFIED)
>> + {
>> + svn_handle_error2(error, stderr, FALSE, "svn: Skipping argument: ");
>> + svn_error_clear(error);
>
> This is making the interesting assumption that any chain containing
> this sort of error *only* contains this sort of error. This
> assumption should be documented in the svn_opt_args_to_target_array3 docstring.
>
>> + }
>> + else
>> + return error;
>> + }
>> + return SVN_NO_ERROR;
>> +}
>> +
>> +
>> /* Helper for svn_cl__get_changelist(); implements
>> svn_changelist_receiver_t. */
>> static svn_error_t *
>>
>
>
>
Received on 2008-02-01 19:48:09 CET