cmpilato@tigris.org writes:
> Modified: trunk/subversion/libsvn_subr/target.c
> ==============================================================================
> --- trunk/subversion/libsvn_subr/target.c (original)
> +++ trunk/subversion/libsvn_subr/target.c Tue Apr 29 15:37:11 2003
> @@ -32,158 +32,202 @@
>
> /*** Code. ***/
>
> +
> +static svn_boolean_t
> +redundancy_check (const char *ancestor,
> + const char *path1,
> + const char *path2,
> + svn_depth_t depth,
> + apr_pool_t *pool)
> +{
This function has no documentation comment.
> + int path1_len;
> + svn_node_kind_t kind;
> + svn_error_t *err;
> +
> + /* For depth zero, there is no redundancy check. */
> + if (depth == svn_depth_zero)
> + return FALSE;
> +
> + /* See if PATH1 is an ancestor of PATH2 */
> + if (strcmp (ancestor, path1) != 0)
> + return FALSE;
> +
> + /* For Depth Infinity, it's enough just to know that PATH1 is an
> + ancestor of PATH2. */
> + if (depth == svn_depth_infinity)
> + return TRUE;
> +
> + /* For Depth 1, we only care if PATH2 is a file immediately inside
> + PATH1. So if the difference of PATH2 and PATH1 contains a
> + directory separator, we know PATH2 is not an immediate child of
> + PATH1. */
> + path1_len = strlen (path1);
> + if (strchr (path2 + path1_len + 1, '/'))
> + return FALSE;
> +
> + /* We now know that PATH2 is an immediate child of PATH1. All that
> + remains is to see if PATH2 is a file or not. If we can't answer
> + that, we'll go the safe route and assume the path is not
> + redundant. */
> + if (! svn_path_is_url (path2))
> + {
> + err = svn_io_check_path (path2, &kind, pool);
This is checking the physical item on the disk, and it's possible for
a physical item to obstruct a versioned item of a different kind,
e.g. an unversioned file could obstruct a versioned directory. Does
it matter that the redundancy will apply to the physical kind rather
than the versioned kind? My guess is that in practice it will not
matter, but it may be worth mentioning it in the documentation for
svn_path_condense_targets.
> + if ((err == SVN_NO_ERROR) && (kind == svn_node_file))
> + return TRUE;
> + if (err)
> + svn_error_clear (err);
> + }
> +
> + return FALSE;
> +}
> +
More importantly, this change appears to introduce a regression as
follows (note the two commit targets)
svnadmin create repo
svn co file://`pwd`/repo wc
echo x >> wc/foo
svn add wc/foo
svn ci wc wc/foo
Using valgrind the commit gives several warnings, I think they all
follow from the first at line 210 in target.c where an invalid string
is added to the array.
Ah, I've discovered target-test, it shows a similar problem, try
valgrind subversion/tests/libsvn_subr/.libs/lt-target-test 0 wc wc/foo
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Apr 30 00:02:30 2003