You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/12/08 14:29:53 UTC

svn commit: r1043408 - in /subversion/trunk/subversion/libsvn_fs_fs: fs_fs.c fs_fs.h

Author: julianfoad
Date: Wed Dec  8 13:29:52 2010
New Revision: 1043408

URL: http://svn.apache.org/viewvc?rev=1043408&view=rev
Log:
Remove the re-try logic from svn_fs_fs__path_rev_absolute().

The re-try logic was introduced in r875097 and reduced but did not eliminate
the window of opportunity for the caller to use an out-of-date result.
Since r1040832, all its callers correctly account for the possibility of an
out-of-date result due to a concurrent packing operation.

See the email thread <http://svn.haxx.se/dev/archive-2010-12/0019.shtml>,
subject "Re: svn commit: r1040832 - Port a fix for a FSFS packing race".

* subversion/libsvn_fs_fs/fs_fs.c
  (svn_fs_fs__path_rev_absolute): Remove the re-try logic.

* subversion/libsvn_fs_fs/fs_fs.h
  (svn_fs_fs__path_rev_absolute): Update the doc string accordingly.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1043408&r1=1043407&r2=1043408&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Dec  8 13:29:52 2010
@@ -254,45 +254,16 @@ svn_fs_fs__path_rev_absolute(const char 
 {
   fs_fs_data_t *ffd = fs->fsap_data;
 
-  if (ffd->format < SVN_FS_FS__MIN_PACKED_FORMAT)
+  if (ffd->format < SVN_FS_FS__MIN_PACKED_FORMAT
+      || ! is_packed_rev(fs, rev))
     {
       *path = path_rev(fs, rev, pool);
-      return SVN_NO_ERROR;
     }
-
-  if (! is_packed_rev(fs, rev))
+  else
     {
-      svn_node_kind_t kind;
-
-      /* Initialize the return variable. */
-      *path = path_rev(fs, rev, pool);
-
-      SVN_ERR(svn_io_check_path(*path, &kind, pool));
-      if (kind == svn_node_file)
-        {
-          /* *path is already set correctly. */
-          return SVN_NO_ERROR;
-        }
-      else
-        {
-          /* Someone must have run 'svnadmin pack' while this fs object
-           * was open. */
-
-          SVN_ERR(update_min_unpacked_rev(fs, pool));
-
-          /* The rev really should be present now. */
-          if (! is_packed_rev(fs, rev))
-            return svn_error_createf(APR_ENOENT, NULL,
-                                     _("Revision file '%s' does not exist, "
-                                       "and r%ld is not packed"),
-                                     svn_dirent_local_style(*path, pool),
-                                     rev);
-          /* Fall through. */
-        }
+      *path = path_rev_packed(fs, rev, "pack", pool);
     }
 
-  *path = path_rev_packed(fs, rev, "pack", pool);
-
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h?rev=1043408&r1=1043407&r2=1043408&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h Wed Dec  8 13:29:52 2010
@@ -411,8 +411,10 @@ svn_error_t *svn_fs_fs__move_into_place(
    Allocate *PATH in POOL.
 
    Note: If the caller does not have the write lock on FS, then the path is
-   not guaranteed to remain correct after the function returns, because the
-   revision might become packed just after this call. */
+   not guaranteed to be correct or to remain correct after the function
+   returns, because the revision might become packed before or after this
+   call.  If a file exists at that path, then it is correct; if not, then
+   the caller should call update_min_unpacked_rev() and re-try once. */
 svn_error_t *
 svn_fs_fs__path_rev_absolute(const char **path,
                              svn_fs_t *fs,