Hi Daniel, Being a non SVN dev I am certainly not in a position to review your patch.. But being a developer, none the less, that prescribes to Test Driven Development I have to say, Why Fight it?
Occam's razor? Beau. On 09/01/2011, at 2:42 AM, Daniel Shahaf wrote: > [ issue #3705 is the "corruption that fsfsverify fixes" issue. ] > > One of my old todo notes from issue #3705 is to flush proto rev files. > Looking into the code, I came up with the following. > > Does that look right? It seems too short to be the real fix. > > [[[ > Index: ../libsvn_fs_fs/fs_fs.c > =================================================================== > --- ../libsvn_fs_fs/fs_fs.c (revision 1056454) > +++ ../libsvn_fs_fs/fs_fs.c (working copy) > @@ -795,8 +795,11 @@ get_writable_proto_rev_body(svn_fs_t *fs, const vo > > /* Now open the prototype revision file and seek to the end. */ > err = svn_io_file_open(file, path_txn_proto_rev(fs, txn_id, pool), > - APR_WRITE | APR_BUFFERED, APR_OS_DEFAULT, pool); > + APR_WRITE, APR_OS_DEFAULT, pool); > > + /* Disable buffering. See notorious issue #3705. */ > + apr_err = apr_file_buffer_set(*file, NULL, 0); > + > /* You might expect that we could dispense with the following seek > and achieve the same thing by opening the file using APR_APPEND. > Unfortunately, APR's buffered file implementation unconditionally > @@ -2029,7 +2032,7 @@ open_and_seek_transaction(apr_file_t **file, > apr_off_t offset; > > SVN_ERR(svn_io_file_open(&rev_file, path_txn_proto_rev(fs, txn_id, pool), > - APR_READ | APR_BUFFERED, APR_OS_DEFAULT, pool)); > + APR_READ, APR_OS_DEFAULT, pool)); > > offset = rep->offset; > SVN_ERR(svn_io_file_seek(rev_file, APR_SET, &offset, pool)); > ]]] > > Daniel > (why disable buffering? I don't remember, but I could refresh my > memory using the issue and IRC logs.)
