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

Re: svn commit: r14851 - trunk/subversion/libsvn_client

From: <kfogel_at_collab.net>
Date: 2005-05-27 17:34:45 CEST

fitz@tigris.org writes:
> Log:
> A few style cleanups, courtesy of cmpilato.
>
> * subversion/libsvn_client/locking_commands.c (store_locks_callback):
> Formatting tweak.
>
> (organize_lock_targets): Formatting tweaks and use apr macros where
> possible. Remove an unnecessary error check.

The log message implies that this commit is basically just formatting
tweaks (I count preprocessor stuff as essentially a formatting tweak),
and one error-check removal.

But there's a substantive change as well: *rel_urls_p now gets set to
NULL at the beginning of the call, before the paths are condensed.
Therefore, it might be better for the log message not to give the
impression that this was just about cosmetics.

There's a more important question as well:

> --- trunk/subversion/libsvn_client/locking_commands.c (original)
> +++ trunk/subversion/libsvn_client/locking_commands.c Thu May 26 14:09:12 2005
> @@ -64,7 +64,7 @@
> 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);
> + char *path = apr_hash_get (lb->urls_to_paths, rel_url, APR_HASH_KEY_STRING);
>
> /* Create the notify struct first, so we can tweak it below. */
> notify = svn_wc_create_notify (rel_url,
> @@ -175,6 +175,9 @@
> apr_array_header_t *rel_targets = apr_array_make(pool, 1,
> sizeof(const char *));
> apr_hash_t *rel_targets_ret = apr_hash_make(pool);
> +
> + *rel_urls_p = NULL;
> +
> /* Get the common parent and all relative paths */
> SVN_ERR (svn_path_condense_targets (common_parent, &rel_targets, targets,
> FALSE, pool));

The doc string for this function says

 * [...]
 *
 * If TARGETS is an array of urls, REL_URLS_P is set to NULL.
 * Otherwise each key in REL_URLS_P is a partial url (relative to
 * COMMON_PARENT) mapped to the target path for TARGET (relative to
 * the PARENT_ENTRY_P). working copy targets that they "belong" to.
 *
 * [...]

But now, *REL_URLS_P gets set to NULL unconditionally, before anything
is discovered about TARGETS. Either the doc string needs to change,
or the code needs to change.

-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:13:17 2005

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