Prabhu, some more review comments on your last patch.

Instead of checking for errors in several places within the main loop, put the 
verification code in a wrapper function and check for errors exactly once where 
you call that function.  That way you won't miss any.  In your last patch, 
there is at least one call that could return a verification error that isn't 
checked -- the call to svn_fs_revision_root().

Run the test suite before posting a patch, and mention the results.  Three of 
the svnadmin tests fail when I run them with your patch:

  FAIL:  svnadmin_tests.py 11: verify a repository containing paths like 'c:hi'
  FAIL:  svnadmin_tests.py 19: svnadmin verify detects invalid revprops file
  FAIL:  svnadmin_tests.py 23: svnadmin verify with non-UTF-8 paths

Look for compiler warnings, and address any that indicate problems.  I see two:

  subversion/libsvn_repos/dump.c:1363:1: no previous prototype for 
'notify_verification_error'
  subversion/libsvn_repos/dump.c:1442:20: declaration of 'err' shadows a 
previous local


Include the log message each time you send the patch, even if the log message 
is the same as it was the previous time, so we don't have to hunt for it.

Thanks.
- Julian

Reply via email to