On Tue, Feb 01, 2011 at 04:42:10PM -0000, cmpil...@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);
> + }
