fitz@tigris.org writes:
> Modified: trunk/subversion/libsvn_client/locking_commands.c
> ==============================================================================
> --- trunk/subversion/libsvn_client/locking_commands.c (original)
> +++ trunk/subversion/libsvn_client/locking_commands.c Thu May 26 11:58:53 2005
[...]
> @@ -63,9 +64,10 @@
> svn_wc_adm_access_t *adm_access;
> const char *abs_path;
> svn_wc_notify_t *notify;
> + char *path = apr_hash_get(lb->urls_to_paths, rel_url, APR_HASH_KEY_STRING);
space-before-paren.
> @@ -200,10 +210,15 @@
> do_lock ? (const void *) invalid_revnum
> : (const void *) "");
> }
> + *rel_urls_p = NULL;
> }
(I'm only looking at the patch, not the context.) Any reason not to
just initialize this at the top of the function?
> else /* common parent is a local path */
> {
> int max_depth = 0;
> + apr_array_header_t *rel_urls;
> + apr_array_header_t *urls = apr_array_make(pool, 1, sizeof(const char *));
> + apr_hash_t *urls_hash = apr_hash_make(pool);
> + const char *common_url;
>
> /* Calculate the maximum number of components in the rel_targets, which
> is the depth to which we need to lock the WC. */
More space-before-paren stuff here.
> @@ -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);
> +
> +
> + }
> +
> + /* 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')
This may just be a personal style thing, but I prefer:
if (! (common_url && *common_url))
> + 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."));
Stray capitalization of "Lock" here. Also, why does this error get an
SVN_ERR_UNSUPPORTED_FEATURE?
> + 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];
Might want to use these macros:
APR_ARRAY_IDX(rel_targets, i, const char *);
APR_ARRAY_IDX(rel_urls, i, const char *);
> + 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];
Here, too (macros).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu May 26 20:23:42 2005