David Glasser wrote:
> There's the following function in fs_fs.c:
> /* Move the file into place. */
> err = svn_io_file_rename(old_filename, new_filename, pool);
> if (err && APR_STATUS_IS_EXDEV(err->apr_err))
> #ifdef __linux__
> SVN_ERR(svn_io_file_open(&file, dirname, APR_READ, APR_OS_DEFAULT,
> SVN_ERR(svn_io_file_flush_to_disk(file, pool));
> SVN_ERR(svn_io_file_close(file, pool));
> return err;
> Let's say that there's an error returned from the rename at the top,
> but it's not EXDEV. And let's say that we're on Linux.
> Now, the error gets returned. But before it gets returned, the
> directory gets opened, flushed, and closed.
> This seems wrong to me; I think after the first block there should be
> an immediate "SVN_ERR(err)", and the end of the function should just
> be "return SVN_NO_ERROR". (Does that sound reasonable?)
Yes, that sounds reasonable.
> Now, assuming that this change is a good idea, the question is, can
> the current code actually have negative side effects? That is, can
> flushing the parent directory when there was some error in moving the
> file ever actually harm the repository? Or is it just an expensive
The directory operations might well fail for the same reason that the file
rename failed (maybe path not found, file system is read-only, ...), which
could result in the function returning with the directory opened, which could
be bad, but I feel almost anything like this that could happen with this wierd
code ordering could also happen without it.
Perhaps if we can't demonstrate a problem it's safer to wait till 1.5 just in
case it's actually serving some purpose the way it is.
But what got me interested enough to reply is:
Should we move this OS-specific knowledge to svn_io (at least, or eventually
APR)? The "flush the directory entry for file F" part in particular, but pretty
much the whole move-file-into-place body of this function probably. Don't we
have something very similar (with not quite so strict flushing) in the WC code?
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Sun Nov 4 00:36:17 2007