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

Re: [PATCH] implement "svn switch --only-rewrite-urls"

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2002-10-01 18:02:21 CEST

mark benedetto king <bking@Inquira.Com> writes:

> Index: subversion/libsvn_wc/relocate.c
> ===================================================================
> --- subversion/libsvn_wc/relocate.c
> +++ subversion/libsvn_wc/relocate.c Mon Sep 30 18:46:07 2002
> @@ -0,0 +1,125 @@
[snip]
> +
> +svn_error_t *
> +svn_wc_relocate (const char *path,
> + svn_wc_adm_access_t *adm_access,
> + const char *from,
> + const char *to,
> + svn_boolean_t recurse,
> + apr_pool_t *pool)
> +{
> + enum svn_node_kind kind;
> + apr_hash_t *entries = NULL;
> + apr_hash_index_t *hi;
> + svn_boolean_t is_file = FALSE;
> + char *base;
> + int from_len;
> +
> + SVN_ERR(svn_io_check_path(path, &kind, pool));
> +
> + if (kind == svn_node_file)
> + {
> + base = svn_path_basename(path, pool);
> + is_file = TRUE;
> + }
> +
> + from_len = strlen(from);
> +
> + SVN_ERR(svn_wc_entries_read(&entries, adm_access, FALSE, pool));
> +
> + if (is_file)
> + {
> + const char *url;
> + svn_wc_entry_t *this_dir = apr_hash_get(entries,
> + SVN_WC_ENTRY_THIS_DIR,
> + APR_HASH_KEY_STRING);
> + svn_wc_entry_t *entry = apr_hash_get(entries, base, APR_HASH_KEY_STRING);
> + if (!this_dir)
> + return svn_error_create(SVN_ERR_ENTRY_NOT_FOUND, 0, NULL, pool,
> + "missing default entry");
> + if (!entry)
> + return svn_error_create(SVN_ERR_ENTRY_NOT_FOUND, 0, NULL, pool,
> + "missing entry");
> +
> + url = entry->url;
> + if (!url)
> + url = svn_path_join(this_dir->url, base, pool);

This still doesn't look right. If the entry didn't originally have an
URL why are you creating one? Why is this logic different from the
logic in the the loop below? If the file doesn't have an URL then I
don't think relocate should create one. Trying to relocate a file
without an URL might perhaps produce an error or it might not, I could
argue either way.

> +
> + if (!strncmp(url, from, from_len))
> + {
> + entry->url = apr_psprintf (pool, "%s%s", to, url + from_len);

Didn't spot this yesterday, but this is not right, see below.

> + SVN_ERR(svn_wc__entries_write (entries, adm_access, pool));
> + }
> +
> + return SVN_NO_ERROR;
> + }
> +
> + for (hi = apr_hash_first(pool, entries); hi; hi = apr_hash_next(hi))
> + {
> + const void *key;
> + void *val;
> + svn_wc_entry_t *entry;
> +
> + apr_hash_this(hi, &key, NULL, &val);
> + entry = val;
> +
> +
> + if (recurse
> + && (entry->kind == svn_node_dir)
> + && (strcmp(key, SVN_WC_ENTRY_THIS_DIR) != 0))
> + {
> + svn_wc_adm_access_t *subdir_access;
> + const char *subdir = svn_path_join (path, key, pool);
> + SVN_ERR(svn_wc_adm_retrieve(&subdir_access, adm_access, subdir,
> + pool));
> + SVN_ERR(svn_wc_relocate(subdir, subdir_access, from,
> + to, recurse, pool));
> + }
> +
> + if (entry->url &&
> + (strncmp(entry->url, from, from_len) == 0))
> + entry->url = apr_psprintf (pool, "%s%s", to, entry->url + from_len);

This has the same problem as the single file apr_psprintf above, it
modifies the access baton's entry cache, but doesn't use the right
pool. Memory used to modify the access baton entry cache must come
from the access baton pool. There are two solutions, either a) use
svn_wc__entry_modify which does its own allocation, or b) use
svn_wc_adm_access_pool to get the access baton pool for use in
apr_psprintf. Solution a) is perhaps simpler for someone reading the
code to understand, but b) is more efficient (at the moment) since it
doesn't rewrite the entries file for each entry modified.

In rev 3232, in most of the places that use svn_wc_entries_read, I
changed the svn_wc_entry_t pointers so that they were const, to avoid
any accidental cache modifications.

Having said that, as you haven't used any subpools the memory
allocation happens to be correct :) However, svn_wc_relocate should
not rely on its POOL argument being suitable for caching in
ADM_ACCESS, it's not guaranteed to work.

> + }
> +
> + SVN_ERR(svn_wc__entries_write (entries, adm_access, pool));
> +
> + return SVN_NO_ERROR;
> +}
> +

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Oct 1 18:03:05 2002

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.