[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r29127 - in trunk/subversion: bindings/swig include libsvn_subr libsvn_wc svn tests/cmdline

From: Augie Fackler <durin42_at_gmail.com>
Date: Fri, 1 Feb 2008 13:09:47 -0600

On Feb 1, 2008, at 12:42 PM, 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.

Whoops - someone already caught that...sorry.

>>
>> /* 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!)

*nod* I'll fix that and the below issue in a single patch to clean
things up.

<snip issues that others fixed>
>> 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.

Is there some better way to do this that might be preferable? I'd be
happy to change svn_opt_args_to_target_array3() to work a bit more
sanely (I didn't see anything else that could return multiple errors,
and nesting them seemed the logical choice).

>> + }
>> + else
>> + return error;
>> + }
>> + return SVN_NO_ERROR;
>> +}
>> +
>> +
>> /* Helper for svn_cl__get_changelist(); implements
>> svn_changelist_receiver_t. */
>> static svn_error_t *
>>

I'll start working on making these two fixes. Can anyone point me to
the "correct" way to return multiple errors from a function?

Thanks,
Augie

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-01 20:10:00 CET

This is an archived mail posted to the Subversion Dev mailing list.