fitz@tigris.org writes:
> Log:
> Fix locking bug that occurs when locking a switched file. The failing
> test was committed in r14774 but is now passing (with minor changes).
Was this issue #2307? Log should say so, if so.
> --- trunk/subversion/libsvn_client/locking_commands.c (original)
> +++ trunk/subversion/libsvn_client/locking_commands.c Thu May 26 11:58:53 2005
> @@ -234,19 +248,19 @@
> _("'%s' has no URL"),
> svn_path_local_style (*common_parent, pool));
>
> - /* Verify all paths. */
> + /* Get the url for each target and verify all paths. */
> for (i = 0; i < rel_targets->nelts; i++)
> {
> const svn_wc_entry_t *entry;
> const char *target = ((const char **) (rel_targets->elts))[i];
> const char *abs_path;
> -
> +
> abs_path = svn_path_join
> (svn_wc_adm_access_path (*parent_adm_access_p), target, pool);
> -
> +
> SVN_ERR (svn_wc_entry (&entry, abs_path, *parent_adm_access_p, FALSE,
> pool));
> -
> +
> if (! entry)
> return svn_error_createf (SVN_ERR_UNVERSIONED_RESOURCE, NULL,
> _("'%s' is not under version control"),
> @@ -255,14 +269,71 @@
> return svn_error_createf (SVN_ERR_ENTRY_MISSING_URL, NULL,
> _("'%s' has no URL"),
> svn_path_local_style (target, pool));
> -
> +
> + (*((const char **)(apr_array_push (urls)))) = apr_pstrdup
> + (pool, entry->url);
> +
> +
> + }
Is there some reason we're not using a subpool for at least the tmp
work in that 'for' loop?
> + /* Condense our absolute urls and get the relative urls. */
> + SVN_ERR (svn_path_condense_targets (&common_url, &rel_urls, urls,
> + FALSE, pool));
> +
> + /* svn_path_condense_targets leaves paths empty if TARGETS only had
> + 1 member, so we special case that (again). */
> + if (apr_is_empty_array (rel_urls))
> + {
> + char *base_name = svn_path_basename (common_url, pool);
> + common_url = svn_path_dirname (common_url, pool);
> + APR_ARRAY_PUSH(rel_urls, char *) = base_name;
> + }
> +
> + /* If we have no common URL parent, bail (cross-repos lock attempt) */
> + if (common_url == NULL || (common_url)[0] == '\0')
> + return svn_error_create
> + (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> + _("Unable to lock/unlock across multiple repositories"));
> +
> + if (rel_urls->nelts != rel_targets->nelts)
> + return svn_error_create
> + (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> + _("Error condensing Lock/unlock targets."));
> +
> + for (i = 0; i < rel_urls->nelts; i++)
> + {
> + const char *target = ((const char **) (rel_targets->elts))[i];
> + const char *rel_url = ((const char **) (rel_urls->elts))[i];
> +
> + apr_hash_set (urls_hash, apr_pstrdup (pool, rel_url),
> + APR_HASH_KEY_STRING,
> + apr_pstrdup (pool, target));
> + }
...I guess no subpool advantage is to be had here, however...
> + *rel_urls_p = urls_hash;
> + *common_parent = common_url;
> +
> + /* Now that we've got the relative URLs, gather our targets. */
> + for (i = 0; i < rel_targets->nelts; i++)
> + {
> + const svn_wc_entry_t *entry;
> + const char *target = ((const char **) (rel_targets->elts))[i];
> + const char *url = ((const char **) (rel_urls->elts))[i];
> + const char *abs_path;
> +
> + abs_path = svn_path_join
> + (svn_wc_adm_access_path (*parent_adm_access_p), target, pool);
> +
> + SVN_ERR (svn_wc_entry (&entry, abs_path, *parent_adm_access_p, FALSE,
> + pool));
> +
> if (do_lock) /* Lock. */
> {
> svn_revnum_t *revnum;
> revnum = apr_palloc (pool, sizeof (* revnum));
> *revnum = entry->revision;
>
> - apr_hash_set (rel_targets_ret, apr_pstrdup (pool, target),
> + apr_hash_set (rel_targets_ret, apr_pstrdup (pool, url),
> APR_HASH_KEY_STRING, revnum);
> }
> else /* Unlock. */
On the other hand, the gather-targets loop could use a subpool for its
tmp operations.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 27 18:05:50 2005