On Tue, Mar 16, 2010 at 09:07, <philip_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_client/client.h Tue Mar 16 13:07:40 2010
> @@ -655,7 +655,8 @@ svn_client__switch_internal(svn_revnum_t
> TARGET is a working-copy path, the base of the hierarchy to be
> compared. It corresponds to the URL opened in RA_SESSION below.
>
> - WC_CTX is a context for the working copy.
> + WC_CTX is a context for the working copy and should be NULL for
> + operations that do not involve a working copy.
This kind of comment should be added to all functions where this is
allowed. Normally wc_ctx!=NULL throughout libsvn_client, so it is a
very different departure to allow this.
>...
> +++ subversion/trunk/subversion/libsvn_client/repos_diff.c Tue Mar 16 13:07:40 2010
> @@ -51,7 +51,7 @@ struct edit_baton {
> const char *target;
>
> /* ADM_ACCESS is an access baton that includes the TARGET directory */
> - svn_wc_adm_access_t *adm_access;
> + svn_wc_context_t *wc_ctx;
The comment should be adjusted to reference WC_CTX, and to note the
NULL possibility.
> @@ -321,56 +321,65 @@ get_dirprops_from_ra(struct dir_baton *b
> }
>
>
> -/* Return in *LOCAL_DIR_ABSPATH the absolute path for the directory PATH by
> - searching the access baton set of ADM_ACCESS. If ADM_ACCESS is NULL then
> - *LOCAL_DIR_ABSPATH will be NULL. If LENIENT is TRUE then failure to find
> - an access baton will not return an error but will set *LOCAL_DIR_ABSPATH to
> - NULL instead. */
> +/* Return in *LOCAL_DIR_ABSPATH the absolute path for the directory
> + PATH if PATH is a versioned directory. If PATH is not a versioned
> + directory and LENIENT is FALSE then return an error
> + SVN_ERR_WC_NOT_WORKING_COPY. If LENIENT is TRUE then failure to
> + find an access baton will not return an error but will set
> + *LOCAL_DIR_ABSPATH to NULL instead.
> +
> + This rather odd interface was originally designed around searching
> + an access baton set. */
"failure to find an access baton" ?? That certainly doesn't make any
sense. The docstring should also mention that if WC_CTX is NULL, then
nothing will ever be returned (*LOCAL_DIR_ABSPATH is set to NULL).
IMO, this function shouldn't even be called in that situation. Callers
should not even be trying functionality which doesn't make sense (ie.
push the WC_CTX check out).
>...
> -/* Like get_path_access except the returned access baton, in
> - *PARENT_ACCESS, is for the parent of PATH rather than for PATH
> - itself. */
> +/* Like get_path_access except the returned path, in
> + *LOCAL_PARENT_DIR_ABSPATH, is for the parent of PATH rather than
> + for PATH itself. */
Add comment about WC_CTX.
>...
Cheers,
-g
Received on 2010-03-16 17:35:14 CET