On Fri, Feb 12, 2010 at 10:39, Matthew Bentham <mjb67_at_artvps.com> wrote:
> I understand from thread "WC-NG: How can we help with 1.7-readiness?" that
> this sort of thing is helpful. I am a massive newbie, please forgive me if
> I do something obviously wrong.
Can't learn without trying! Thanks for jumping in :-)
> [[[
> wc-ng: work towards eliminating svn_wc_entry_t
>
> * subversion/libsvn_client/commit_util.c
> (add_lock_token): Replace a use of svn_wc__maybe_get_entry with
> use of svn_wc__node_get_*
> ]]]
For log messages, we also like to provide attribution, so at the end
of the above message you would have:
Patch by: Matthew Bentham <mjb67{_AT_}artvps.com>
> Index: subversion/libsvn_client/commit_util.c
> ===================================================================
> --- subversion/libsvn_client/commit_util.c (revision 909397)
> +++ subversion/libsvn_client/commit_util.c (working copy)
> @@ -195,19 +195,21 @@
> {
> struct add_lock_token_baton *altb = walk_baton;
> apr_pool_t *token_pool = apr_hash_pool_get(altb->lock_tokens);
> - const svn_wc_entry_t *entry;
> + const char* lock_token = NULL;
> + const char* url = NULL;
No need to initialize these to NULL. The node functions will always
set the value, unless it returns an error (in which case, you don't
care about the values).
> + SVN_ERR(svn_wc__node_get_url(&url, altb->wc_ctx, local_abspath,
> + scratch_pool, scratch_pool));
> + SVN_ERR(svn_wc__node_get_lock_token(&lock_token, altb->wc_ctx,
> + local_abspath, scratch_pool, scratch_pool));
>
> - SVN_ERR(svn_wc__maybe_get_entry(&entry, altb->wc_ctx, local_abspath,
> - svn_node_unknown, FALSE, FALSE,
> - scratch_pool, scratch_pool));
> -
> /* I want every lock-token I can get my dirty hands on!
> If this entry is switched, so what. We will send an irrelevant lock
> token. */
> - if (entry && entry->url && entry->lock_token)
> - apr_hash_set(altb->lock_tokens, apr_pstrdup(token_pool, entry->url),
> + if (url && lock_token)
> + apr_hash_set(altb->lock_tokens, apr_pstrdup(token_pool, url),
> APR_HASH_KEY_STRING,
> - apr_pstrdup(token_pool, entry->lock_token));
> + apr_pstrdup(token_pool, lock_token));
Rather than dup'ing into the token_pool, you could pass that pool as
the result_pool to the node functions.
However, I'd go ahead and suggest leaving it as above so that
token_pool won't grow if only one of the values is present.
Hmm. I would also rejigger things a bit while you're at it: fetch the
lock token first, and if NULL, then early-exit from the function. The
URL can usually be derived, so will be present quite often. The lock
token is the more obvious discriminator. Also, when you fetch the URL,
you'll be able to use token_pool for the call's result_pool. Does that
all make sense?
Cheers,
-g
Received on 2010-02-12 17:11:32 CET