svn commit: r1717876 - /subversion/branches/1.9.x-fsfs-rep-comparison/subversion/libsvn_fs_fs/fs_fs.c

[st...@apache.org]

Author: stefan2
Date: Thu Dec  3 23:37:08 2015
New Revision: 1717876

URL: http://svn.apache.org/viewvc?rev=1717876&view=rev
Log:
On the 1.9.x-fsfs-rep-comparison branch:
Fix the issue that r1712927 on /trunk was meant to fix.

Under the conditions of issue #4554, EXPANDED_SIZE is unreliable and
cannot be used to detect empty representations.  Therefore, we can't
use it in svn_fs_fs__file_text_rep_equal to test for NULL rep / empty
rep equivalence.

* subversion/libsvn_fs_fs/fs_fs.c
  (svn_fs_fs__file_text_rep_equal): If only one rep is NULL, we perform a
                                    full comparison.  NULL reps are rare,
                                    so this is not a performance issue. 

Modified:
    subversion/branches/1.9.x-fsfs-rep-comparison/subversion/libsvn_fs_fs/fs_fs.c

Modified: subversion/branches/1.9.x-fsfs-rep-comparison/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x-fsfs-rep-comparison/subversion/libsvn_fs_fs/fs_fs.c?rev=1717876&r1=1717875&r2=1717876&view=diff
==============================================================================
--- subversion/branches/1.9.x-fsfs-rep-comparison/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/branches/1.9.x-fsfs-rep-comparison/subversion/libsvn_fs_fs/fs_fs.c Thu Dec  3 23:37:08 2015
@@ -1443,8 +1443,8 @@ svn_fs_fs__file_text_rep_equal(svn_boole
   svn_stream_t *contents_a, *contents_b;
   representation_t *rep_a = a->data_rep;
   representation_t *rep_b = b->data_rep;
-  svn_boolean_t a_empty = !rep_a || rep_a->expanded_size == 0;
-  svn_boolean_t b_empty = !rep_b || rep_b->expanded_size == 0;
+  svn_boolean_t a_empty = !rep_a;
+  svn_boolean_t b_empty = !rep_b;
 
   /* This makes sure that neither rep will be NULL later on */
   if (a_empty && b_empty)
@@ -1453,28 +1453,6 @@ svn_fs_fs__file_text_rep_equal(svn_boole
       return SVN_NO_ERROR;
     }
 
-  if (a_empty != b_empty)
-    {
-      *equal = FALSE;
-      return SVN_NO_ERROR;
-    }
-
-  /* File text representations always know their checksums - even in a txn. */
-  if (memcmp(rep_a->md5_digest, rep_b->md5_digest, sizeof(rep_a->md5_digest)))
-    {
-      *equal = FALSE;
-      return SVN_NO_ERROR;
-    }
-
-  /* Paranoia. Compare SHA1 checksums because that's the level of
-     confidence we require for e.g. the working copy. */
-  if (rep_a->has_sha1 && rep_b->has_sha1)
-    {
-      *equal = memcmp(rep_a->sha1_digest, rep_b->sha1_digest,
-                      sizeof(rep_a->sha1_digest)) == 0;
-      return SVN_NO_ERROR;
-    }
-
   /* Same path in same rev or txn? */
   if (svn_fs_fs__id_eq(a->id, b->id))
     {
@@ -1482,6 +1460,30 @@ svn_fs_fs__file_text_rep_equal(svn_boole
       return SVN_NO_ERROR;
     }
 
+  /* Beware of the combination NULL rep and possibly empty rep.
+   * Due to EXPANDED_SIZE not being reliable, we can't easily detect empty
+   * reps. So, we can only take further shortcuts if both reps are given. */
+  if (!a_empty && !b_empty)
+    {
+      /* File text representations always know their checksums -
+       * even in a txn. */
+      if (memcmp(rep_a->md5_digest, rep_b->md5_digest,
+                 sizeof(rep_a->md5_digest)))
+        {
+          *equal = FALSE;
+          return SVN_NO_ERROR;
+        }
+
+      /* Paranoia. Compare SHA1 checksums because that's the level of
+         confidence we require for e.g. the working copy. */
+      if (rep_a->has_sha1 && rep_b->has_sha1)
+        {
+          *equal = memcmp(rep_a->sha1_digest, rep_b->sha1_digest,
+                          sizeof(rep_a->sha1_digest)) == 0;
+          return SVN_NO_ERROR;
+        }
+    }
+
   SVN_ERR(svn_fs_fs__get_contents(&contents_a, fs, rep_a, TRUE,
                                   scratch_pool));
   SVN_ERR(svn_fs_fs__get_contents(&contents_b, fs, rep_b, TRUE,