You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/07/05 17:57:39 UTC
Re: svn commit: r38295 - in trunk/subversion: include libsvn_wc
On Wed, Jul 1, 2009 at 19:21, Hyrum K. Wright<hy...@hyrumwright.org> wrote:
>...
> +++ trunk/subversion/include/svn_wc.h Wed Jul 1 10:21:12 2009 (r38295)
> @@ -4536,12 +4536,27 @@ svn_wc_get_switch_editor(svn_revnum_t *t
>
> /** Set @a *props to a hash table mapping <tt>char *</tt> names onto
> * <tt>svn_string_t *</tt> values for all the regular properties of
> - * @a path. Allocate the table, names, and values in @a pool. If
> - * the node has no properties, or does not exist in the working copy,
> - * then an empty hash is returned. @a adm_access is an access baton
> - * set that contains @a path.
> + * @a local_abspath. Allocate the table, names, and values in
> + * @a result_pool. If the node has no properties, or does not exist in
> + * the working copy, then an empty hash is returned. Use @a wc_ctx to
> + * access the working copy, and @a scratch_pool for temporary allocations.
> + *
> + * @since New in 1.7.
> */
Since the API is being rev'd, then let's get this right: if the node
does not exist, then throw an error.
Callers shouldn't be using this API for missing nodes. They want
properties, then fine... but the node should be there to fetch
properties *from*.
This will also allow callers to distinguish between "no props" and
"does not exist".
>...
> +++ trunk/subversion/libsvn_wc/props.c Wed Jul 1 10:21:12 2009 (r38295)
> @@ -1759,26 +1759,26 @@ svn_wc__wcprop_set(svn_wc__db_t *db,
>
>
> svn_error_t *
> -svn_wc_prop_list(apr_hash_t **props,
> - const char *path,
> - svn_wc_adm_access_t *adm_access,
> - apr_pool_t *pool)
> +svn_wc_prop_list2(apr_hash_t **props,
> + svn_wc_context_t *wc_ctx,
> + const char *local_abspath,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> {
> - svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
> - const char *local_abspath;
> svn_wc__db_kind_t kind;
>
> /* if there is no entry, 'path' is not under version control and
> therefore has no props. */
> - SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
How about an assertion that the caller gave us an abspath?
> - SVN_ERR(svn_wc__db_check_node(&kind, db, local_abspath, pool));
> + SVN_ERR(svn_wc__db_check_node(&kind, wc_ctx->db, local_abspath,
> + scratch_pool));
> if (kind == svn_wc__db_kind_unknown)
> {
> - *props = apr_hash_make(pool);
> + *props = apr_hash_make(result_pool);
> return SVN_NO_ERROR;
> }
This is the part that should go. Just try to load the props, and fail
if they aren't there.
> - return svn_wc__load_props(NULL, props, NULL, db, local_abspath, pool, pool);
> + return svn_wc__load_props(NULL, props, NULL, wc_ctx->db, local_abspath,
> + result_pool, scratch_pool);
I'm assuming this errors if the node is missing.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2368166
Re: svn commit: r38295 - in trunk/subversion: include libsvn_wc
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On Jul 5, 2009, at 12:57 PM, Greg Stein wrote:
> On Wed, Jul 1, 2009 at 19:21, Hyrum K. Wright<hy...@hyrumwright.org>
> wrote:
>> ...
>> +++ trunk/subversion/include/svn_wc.h Wed Jul 1 10:21:12
>> 2009 (r38295)
>> @@ -4536,12 +4536,27 @@ svn_wc_get_switch_editor(svn_revnum_t *t
>>
>> /** Set @a *props to a hash table mapping <tt>char *</tt> names onto
>> * <tt>svn_string_t *</tt> values for all the regular properties of
>> - * @a path. Allocate the table, names, and values in @a pool. If
>> - * the node has no properties, or does not exist in the working
>> copy,
>> - * then an empty hash is returned. @a adm_access is an access baton
>> - * set that contains @a path.
>> + * @a local_abspath. Allocate the table, names, and values in
>> + * @a result_pool. If the node has no properties, or does not
>> exist in
>> + * the working copy, then an empty hash is returned. Use @a
>> wc_ctx to
>> + * access the working copy, and @a scratch_pool for temporary
>> allocations.
>> + *
>> + * @since New in 1.7.
>> */
>
> Since the API is being rev'd, then let's get this right: if the node
> does not exist, then throw an error.
>
> Callers shouldn't be using this API for missing nodes. They want
> properties, then fine... but the node should be there to fetch
> properties *from*.
>
> This will also allow callers to distinguish between "no props" and
> "does not exist".
r39203.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2392862