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

Re: svn commit: r14849 - in trunk/subversion: tests/clients/cmdline

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: 2005-05-26 20:22:13 CEST

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

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