Sergey Raevskiy <sergey.raevs...@visualsvn.com> writes: > I've attached the patch with crashing test and simple fix, but I'm not really > sure about my solution. A probably better approach would be to replace magic > pointer value by an empty string, but I'm not sure about binary compatibility > etc.
The empty string is a valid filesystem path, although not one we can currently lock. The documentation for svn_fs_access_t is wrong here, it states the lock_tokens hash stores a mapping of UUID-->1 but it really stores UUID-->path where path can be the magic value 1. At present our backend does not check explicilty check the path but if a backend did check the path then changing the magic 1 to an empty string is probably the wrong thing to do. I wonder if we should be checking the path? The modified test below still passes, is that what we want? Index: subversion/tests/libsvn_fs/locks-test.c =================================================================== --- subversion/tests/libsvn_fs/locks-test.c (revision 1657760) +++ subversion/tests/libsvn_fs/locks-test.c (working copy) @@ -531,7 +531,8 @@ final_lock_check(const svn_test_opts_t *opts, /* Supply correct username and token; commit should work. */ SVN_ERR(svn_fs_set_access(fs, access)); - SVN_ERR(svn_fs_access_add_lock_token(access, mylock->token)); + /* Wrong path but commit still works! */ + SVN_ERR(svn_fs_access_add_lock_token2(access, "/made/up", mylock->token)); SVN_ERR(svn_fs_commit_txn(&conflict, &newrev, txn, pool)); SVN_TEST_ASSERT(SVN_IS_VALID_REVNUM(newrev)); -- Philip Martin | Subversion Committer WANdisco // *Non-Stop Data*
