You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2011/02/01 17:42:10 UTC
svn commit: r1066087 - in /subversion/trunk/subversion/libsvn_subr:
dirent_uri.c target.c
Author: cmpilato
Date: Tue Feb 1 16:42:10 2011
New Revision: 1066087
URL: http://svn.apache.org/viewvc?rev=1066087&view=rev
Log:
Upgrade some uses of deprecated path functions to the new
dirent/uri/etc. interfaces.
* subversion/libsvn_subr/dirent_uri.c
(svn_uri_get_longest_ancestor)
* subversion/libsvn_subr/target.c
(svn_path_condense_targets, svn_path_remove_redundancies)
Modified:
subversion/trunk/subversion/libsvn_subr/dirent_uri.c
subversion/trunk/subversion/libsvn_subr/target.c
Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dirent_uri.c?rev=1066087&r1=1066086&r2=1066087&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
+++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Tue Feb 1 16:42:10 2011
@@ -1407,8 +1407,8 @@ svn_uri_get_longest_ancestor(const char
apr_size_t uri_ancestor_len;
apr_size_t i = 0;
- assert(svn_path_is_canonical(uri1, NULL));
- assert(svn_path_is_canonical(uri2, NULL));
+ assert(svn_uri_is_canonical(uri1, NULL));
+ assert(svn_uri_is_canonical(uri2, NULL));
/* Find ':' */
while (1)
Modified: subversion/trunk/subversion/libsvn_subr/target.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/target.c?rev=1066087&r1=1066086&r2=1066087&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/target.c (original)
+++ subversion/trunk/subversion/libsvn_subr/target.c Tue Feb 1 16:42:10 2011
@@ -47,6 +47,8 @@ svn_path_condense_targets(const char **p
svn_boolean_t *removed;
apr_array_header_t *abs_targets;
int basedir_len;
+ const char *first_target;
+ svn_boolean_t first_target_is_url;
/* Early exit when there's no data to work on. */
if (targets->nelts <= 0)
@@ -58,9 +60,12 @@ svn_path_condense_targets(const char **p
}
/* Get the absolute path of the first target. */
- SVN_ERR(svn_path_get_absolute(pcommon,
- APR_ARRAY_IDX(targets, 0, const char *),
- pool));
+ first_target = APR_ARRAY_IDX(targets, 0, const char *);
+ first_target_is_url = svn_path_is_url(first_target);
+ if (first_target_is_url)
+ first_target = apr_pstrdup(pool, first_target);
+ else
+ SVN_ERR(svn_dirent_get_absolute(pcommon, first_target, pool));
/* Early exit when there's only one path to work on. */
if (targets->nelts == 1)
@@ -88,9 +93,31 @@ svn_path_condense_targets(const char **p
{
const char *rel = APR_ARRAY_IDX(targets, i, const char *);
const char *absolute;
- SVN_ERR(svn_path_get_absolute(&absolute, rel, pool));
+ svn_boolean_t is_url = svn_path_is_url(rel);
+
+ if (is_url)
+ absolute = apr_pstrdup(pool, rel); /* ### TODO: avoid pool dup? */
+ else
+ SVN_ERR(svn_dirent_get_absolute(&absolute, rel, pool));
+
APR_ARRAY_PUSH(abs_targets, const char *) = absolute;
- *pcommon = svn_path_get_longest_ancestor(*pcommon, absolute, pool);
+
+ /* If we've not already determined that there's no common
+ parent, then continue trying to do so. */
+ if (*pcommon && **pcommon)
+ {
+ /* If the is-url-ness of this target doesn't match that of
+ the first target, our search for a common ancestor can
+ end right here. Otherwise, use the appropriate
+ get-longest-ancestor function per the path type. */
+ if (is_url != first_target_is_url)
+ *pcommon = "";
+ else if (first_target_is_url)
+ *pcommon = svn_uri_get_longest_ancestor(*pcommon, absolute, pool);
+ else
+ *pcommon = svn_dirent_get_longest_ancestor(*pcommon, absolute,
+ pool);
+ }
}
if (pcondensed_targets != NULL)
@@ -113,6 +140,7 @@ svn_path_condense_targets(const char **p
{
const char *abs_targets_i;
const char *abs_targets_j;
+ svn_boolean_t i_is_url, j_is_url;
const char *ancestor;
if (removed[j])
@@ -120,9 +148,20 @@ svn_path_condense_targets(const char **p
abs_targets_i = APR_ARRAY_IDX(abs_targets, i, const char *);
abs_targets_j = APR_ARRAY_IDX(abs_targets, j, const char *);
+ i_is_url = svn_path_is_url(abs_targets_i);
+ j_is_url = svn_path_is_url(abs_targets_j);
- ancestor = svn_path_get_longest_ancestor
- (abs_targets_i, abs_targets_j, pool);
+ if (i_is_url != j_is_url)
+ continue;
+
+ if (i_is_url)
+ ancestor = svn_uri_get_longest_ancestor(abs_targets_i,
+ abs_targets_j,
+ pool);
+ else
+ ancestor = svn_dirent_get_longest_ancestor(abs_targets_i,
+ abs_targets_j,
+ pool);
if (*ancestor == '\0')
continue;
@@ -237,10 +276,14 @@ svn_path_remove_redundancies(apr_array_h
const char *rel_path = APR_ARRAY_IDX(targets, i, const char *);
const char *abs_path;
int j;
- svn_boolean_t keep_me;
+ svn_boolean_t is_url, keep_me;
/* Get the absolute path for this target. */
- SVN_ERR(svn_path_get_absolute(&abs_path, rel_path, temp_pool));
+ is_url = svn_path_is_url(rel_path);
+ if (is_url)
+ abs_path = rel_path;
+ else
+ SVN_ERR(svn_dirent_get_absolute(&abs_path, rel_path, temp_pool));
/* For each keeper in ABS_TARGETS, see if this target is the
same as or a child of that keeper. */
@@ -248,6 +291,14 @@ svn_path_remove_redundancies(apr_array_h
for (j = 0; j < abs_targets->nelts; j++)
{
const char *keeper = APR_ARRAY_IDX(abs_targets, j, const char *);
+ svn_boolean_t keeper_is_url = svn_path_is_url(keeper);
+ const char *child_relpath;
+
+ /* If KEEPER hasn't the same is-url-ness as ABS_PATH, we
+ know they aren't equal and that one isn't the child of
+ the other. */
+ if (is_url != keeper_is_url)
+ continue;
/* Quit here if we find this path already in the keepers. */
if (strcmp(keeper, abs_path) == 0)
@@ -257,7 +308,11 @@ svn_path_remove_redundancies(apr_array_h
}
/* Quit here if this path is a child of one of the keepers. */
- if (svn_path_is_child(keeper, abs_path, temp_pool))
+ if (is_url)
+ child_relpath = svn_uri_is_child(keeper, abs_path, temp_pool);
+ else
+ child_relpath = svn_dirent_is_child(keeper, abs_path, temp_pool);
+ if (child_relpath)
{
keep_me = FALSE;
break;
Re: svn commit: r1066087 - in
/subversion/trunk/subversion/libsvn_subr: dirent_uri.c target.c
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Feb 01, 2011 at 08:14:13PM +0100, Stefan Sperling wrote:
> And here, we use it:
>
> > + *pcommon = svn_uri_get_longest_ancestor(*pcommon, absolute, pool);
>
> See http://ci.apache.org/builders/svn-slik-w2k3-x64-ra/builds/1628/steps/Test%20fsfs%2Bserf/logs/faillog
Actually, that link doesn't provide useful information.
Here's how it failed:
0x0000000203697195 in svn_uri_get_longest_ancestor (
uri1=0x2012d3c48 "/home/stsp/svn/svn-trunk/subversion/tests/libsvn_subr/z/A/file",
uri2=0x20313a060 "http://host/A/C/D", pool=0x20c597028)
at subversion/libsvn_subr/dirent_uri.c:1410
1410 assert(svn_uri_is_canonical(uri1, NULL));
> Now we could crash. But in the failing test *pcommon happens to
> point to some string from a previous invocation of this function.
> We ended up using that value, which happened to be a dirent,
> causing the assertion failure. Fixed in r1066143.
>
> > + else
> > + *pcommon = svn_dirent_get_longest_ancestor(*pcommon, absolute,
> > + pool);
> > + }
Re: svn commit: r1066087 - in
/subversion/trunk/subversion/libsvn_subr: dirent_uri.c target.c
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Feb 01, 2011 at 04:42:10PM -0000, cmpilato@apache.org wrote:
> Author: cmpilato
> Date: Tue Feb 1 16:42:10 2011
> New Revision: 1066087
>
> URL: http://svn.apache.org/viewvc?rev=1066087&view=rev
> Log:
> Upgrade some uses of deprecated path functions to the new
> dirent/uri/etc. interfaces.
>
> * subversion/libsvn_subr/dirent_uri.c
> (svn_uri_get_longest_ancestor)
> * subversion/libsvn_subr/target.c
> (svn_path_condense_targets, svn_path_remove_redundancies)
> @@ -58,9 +60,12 @@ svn_path_condense_targets(const char **p
> }
Before this change, *pcommon was always initialised:
> /* Get the absolute path of the first target. */
> - SVN_ERR(svn_path_get_absolute(pcommon,
> - APR_ARRAY_IDX(targets, 0, const char *),
> - pool));
> + first_target = APR_ARRAY_IDX(targets, 0, const char *);
> + first_target_is_url = svn_path_is_url(first_target);
> + if (first_target_is_url)
> + first_target = apr_pstrdup(pool, first_target);
Now, we don't initialise it anymore if the first target is a URL!
> + else
> + SVN_ERR(svn_dirent_get_absolute(pcommon, first_target, pool));
>
> /* Early exit when there's only one path to work on. */
> if (targets->nelts == 1)
> @@ -88,9 +93,31 @@ svn_path_condense_targets(const char **p
> {
> const char *rel = APR_ARRAY_IDX(targets, i, const char *);
> const char *absolute;
> - SVN_ERR(svn_path_get_absolute(&absolute, rel, pool));
> + svn_boolean_t is_url = svn_path_is_url(rel);
> +
> + if (is_url)
> + absolute = apr_pstrdup(pool, rel); /* ### TODO: avoid pool dup? */
> + else
> + SVN_ERR(svn_dirent_get_absolute(&absolute, rel, pool));
> +
> APR_ARRAY_PUSH(abs_targets, const char *) = absolute;
> - *pcommon = svn_path_get_longest_ancestor(*pcommon, absolute, pool);
> +
> + /* If we've not already determined that there's no common
> + parent, then continue trying to do so. */
> + if (*pcommon && **pcommon)
> + {
> + /* If the is-url-ness of this target doesn't match that of
> + the first target, our search for a common ancestor can
> + end right here. Otherwise, use the appropriate
> + get-longest-ancestor function per the path type. */
> + if (is_url != first_target_is_url)
> + *pcommon = "";
> + else if (first_target_is_url)
And here, we use it:
> + *pcommon = svn_uri_get_longest_ancestor(*pcommon, absolute, pool);
See http://ci.apache.org/builders/svn-slik-w2k3-x64-ra/builds/1628/steps/Test%20fsfs%2Bserf/logs/faillog
Now we could crash. But in the failing test *pcommon happens to
point to some string from a previous invocation of this function.
We ended up using that value, which happened to be a dirent,
causing the assertion failure. Fixed in r1066143.
> + else
> + *pcommon = svn_dirent_get_longest_ancestor(*pcommon, absolute,
> + pool);
> + }