Re: valgrind warning in BDB locks code

Philip Martin <philip.martin_at_wandisco.com> writes:

> I'm not 100%
> sure whether we need second one as well as the first one: I suspect that
> the first alone is not enough, but so far I've failed to come up with a
> test case to prove that.

I can't get valgrind to trigger but if I extend the test case like this:

Index: subversion/tests/libsvn_fs/locks-test.c
===================================================================
--- subversion/tests/libsvn_fs/locks-test.c (revision 1205918)
+++ subversion/tests/libsvn_fs/locks-test.c (working copy)
@@ -358,6 +358,16 @@ get_locks(const svn_test_opts_t *opts,
                                        num_expected_paths, pool));
   }
 
+ {
+ static const char *expected_paths[] = { 0 };
+ num_expected_paths = 0;
+ get_locks_baton = make_get_locks_baton(pool);
+ SVN_ERR(svn_fs_get_locks(fs, "A/D/H/ABCDEFGHIJKLMNOPQR", get_locks_callback,
+ get_locks_baton, pool));
+ SVN_ERR(verify_matching_lock_paths(get_locks_baton, expected_paths,
+ num_expected_paths, pool));
+ }
+
   return SVN_NO_ERROR;
 }

then I reach this bit of code:

261 while ((! db_err)
(gdb) n
262 && strncmp(lookup_path, key.data, strlen(lookup_path)) == 0)
(gdb) p lookup_path
$1 = 0x7470d0 "/A/D/H/ABCDEFGHIJKLMNOPQR/"
(gdb) p key
$2 = {data = 0x746bd0, size = 10, ulen = 0, dlen = 0, doff = 0,
  app_data = 0x0, flags = 16}
(gdb) p (char*)key.data
$3 = 0x746bd0 "/A/D/H/chi\354\366\377\177"

and passing that key.data to strncmp, with n>key.size, looks like an
error. So I believe the first fix alone is not enough.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com