Philip Martin wrote:
> I haven't been running the tests much recently, but revert_tests 15
> and 17 have been failing over ra_local for a couple of days at least.
> The is_canonical assertion is failing in svn_path_join like this:
>
> (gdb) up
> #4 0x00002ae7bcfe56aa in svn_ra_local__get_session_url (session=0x5435b0,
> url=0x7fffeedefbe8, pool=0x5432f8)
> at ../src/subversion/libsvn_ra_local/ra_plugin.c:330
> 330 *url = svn_path_join(baton->repos_url,
> (gdb) list
> 325 apr_pool_t *pool)
> 326 {
> 327 svn_ra_local__session_baton_t *baton = session->priv;
> 328 const char *fs_path = apr_pstrmemdup(pool, baton->fs_path->data,
> 329 baton->fs_path->len);
> 330 *url = svn_path_join(baton->repos_url,
> 331 svn_path_uri_encode(fs_path + 1, pool),
> 332 pool);
> 333 return SVN_NO_ERROR;
> 334 }
> (gdb) p baton->fs_path
> $1 = (svn_stringbuf_t *) 0x56b900
> (gdb) p baton->fs_path[0]
> $2 = {pool = 0x5432f8, data = 0x56b8f8 "", len = 0, blocksize = 1}
>
> Note that baton->fs_path is "" and yet fs_path+1 is being passed to
> svn_path_uri_encode, that's obviously wrong. The apr_pstrmemdup call
> looks unnecessary as well. Should this be fixed in ra_local or in tha
> caller? Are other people seeing these failures?
FS_PATH either should or should not have a leading slash -- inconsistency
here is madness. If it has a leading slash, then the current code should be
correct, and some caller is perhaps passing something bogus to the library.
If it does *not* have a leading slash, then we should of course never try
to advance past such a thing with a '+ 1' (which means your patch is incorrect).
I'm not seeing any failures, and the documentation doesn't say for sure, but
I think FS_PATH today *does* have a leading slash. Many RA-API-implementing
functions in libsvn_ra_local build an "absolute filesystem path" by joining
the relative path provided via the RA API to the FS_PATH stored in the
session baton. Some of them just do the join; some do the join but first
transform an empty FS_PATH into "/". It's all very inconsistent, and this
inconsistency is not aided by the fact that the FS and REPOS APIs accept
filesystem paths with or without leading slashes.
My suggestion: decide whether or not FS_PATH does or doesn't have a leading
slash, document in the session baton that decision, and enforce this in
svn_ra_local__split_URL() and svn_ra_local__reparent().
--
C. Michael Pilato <cmpilato@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on Tue Nov 6 18:31:05 2007