You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/05/22 21:08:17 UTC

svn commit: r1485342 - in /subversion/branches/1.7.x: ./ STATUS subversion/libsvn_diff/diff_file.c

Author: stsp
Date: Wed May 22 19:08:17 2013
New Revision: 1485342

URL: http://svn.apache.org/r1485342
Log:
Merge the 1.7.x-issue4270 branch back into 1.7.x.

 * r1427197, r1427210
   Fix issue #4270 'diff -cN shows wrong line changed'.  In the optimized
   code path (unaligned access) for prefix/suffix scanning, the diff would
   be incorrect when a '\r' is not immediately followed by a '\n'.
   Justification:
     Diff should show the correct changes. Failure to do so can lead to
     incorrect updates/merges (because diff3 suffers from the same bug).
   Branch:
     ^/subversion/branches/1.7.x-issue4270
   Votes:
     +1: jcorvel, rhuijben, stsp

Modified:
    subversion/branches/1.7.x/   (props changed)
    subversion/branches/1.7.x/STATUS
    subversion/branches/1.7.x/subversion/libsvn_diff/diff_file.c

Propchange: subversion/branches/1.7.x/
------------------------------------------------------------------------------
  Merged /subversion/trunk:r1427197,1427210
  Merged /subversion/branches/1.7.x-issue4270:r1433737-1485341

Modified: subversion/branches/1.7.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?rev=1485342&r1=1485341&r2=1485342&view=diff
==============================================================================
--- subversion/branches/1.7.x/STATUS (original)
+++ subversion/branches/1.7.x/STATUS Wed May 22 19:08:17 2013
@@ -230,15 +230,3 @@ Veto-blocked changes:
 
 Approved changes:
 =================
-
- * r1427197, r1427210
-   Fix issue #4270 'diff -cN shows wrong line changed'.  In the optimized
-   code path (unaligned access) for prefix/suffix scanning, the diff would
-   be incorrect when a '\r' is not immediately followed by a '\n'.
-   Justification:
-     Diff should show the correct changes. Failure to do so can lead to
-     incorrect updates/merges (because diff3 suffers from the same bug).
-   Branch:
-     ^/subversion/branches/1.7.x-issue4270
-   Votes:
-     +1: jcorvel, rhuijben, stsp

Modified: subversion/branches/1.7.x/subversion/libsvn_diff/diff_file.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/subversion/libsvn_diff/diff_file.c?rev=1485342&r1=1485341&r2=1485342&view=diff
==============================================================================
--- subversion/branches/1.7.x/subversion/libsvn_diff/diff_file.c (original)
+++ subversion/branches/1.7.x/subversion/libsvn_diff/diff_file.c Wed May 22 19:08:17 2013
@@ -433,7 +433,7 @@ find_identical_prefix(svn_boolean_t *rea
         }
 
       is_match = TRUE;
-      for (delta = 0; delta < max_delta && is_match; delta += sizeof(apr_uintptr_t))
+      for (delta = 0; delta < max_delta; delta += sizeof(apr_uintptr_t))
         {
           apr_uintptr_t chunk = *(const apr_size_t *)(file[0].curp + delta);
           if (contains_eol(chunk))
@@ -443,18 +443,25 @@ find_identical_prefix(svn_boolean_t *rea
             if (chunk != *(const apr_size_t *)(file[i].curp + delta))
               {
                 is_match = FALSE;
-                delta -= sizeof(apr_size_t);
                 break;
               }
+
+          if (! is_match)
+            break;
         }
 
-      /* We either found a mismatch or an EOL at or shortly behind curp+delta
-       * or we cannot proceed with chunky ops without exceeding endp.
-       * In any way, everything up to curp + delta is equal and not an EOL.
-       */
-      for (i = 0; i < file_len; i++)
-        file[i].curp += delta;
+      if (delta /* > 0*/)
+        {
+          /* We either found a mismatch or an EOL at or shortly behind curp+delta
+           * or we cannot proceed with chunky ops without exceeding endp.
+           * In any way, everything up to curp + delta is equal and not an EOL.
+           */
+          for (i = 0; i < file_len; i++)
+            file[i].curp += delta;
 
+          /* Skipped data without EOL markers, so last char was not a CR. */
+          had_cr = FALSE; 
+        }
 #endif
 
       *reached_one_eof = is_one_at_eof(file, file_len);
@@ -634,32 +641,41 @@ find_identical_suffix(apr_off_t *suffix_
         can_read_word = can_read_word
                         && (   file_for_suffix[i].curp - sizeof(apr_uintptr_t)
                             >= min_curp[i]);
-      if (can_read_word)
+      while (can_read_word)
         {
-          do
-            {
-              apr_uintptr_t chunk;
-              for (i = 0; i < file_len; i++)
-                file_for_suffix[i].curp -= sizeof(apr_uintptr_t);
+          apr_uintptr_t chunk;
 
-              chunk = *(const apr_uintptr_t *)(file_for_suffix[0].curp + 1);
-              if (contains_eol(chunk))
-                break;
+          /* For each file curp is positioned at the next byte to read, but we
+             want to examine the bytes before the current location. */
 
-              for (i = 0, can_read_word = TRUE; i < file_len; i++)
-                can_read_word = can_read_word
-                                && (   file_for_suffix[i].curp - sizeof(apr_uintptr_t)
-                                    >= min_curp[i]);
-              for (i = 1, is_match = TRUE; i < file_len; i++)
-                is_match = is_match
-                           && (   chunk
-                               == *(const apr_uintptr_t *)(file_for_suffix[i].curp + 1));
-            } while (can_read_word && is_match);
+          chunk = *(const apr_uintptr_t *)(file_for_suffix[0].curp + 1
+                                             - sizeof(apr_uintptr_t));
+          if (contains_eol(chunk))
+            break;
 
-            for (i = 0; i < file_len; i++)
-              file_for_suffix[i].curp += sizeof(apr_uintptr_t);
-        }
+          for (i = 1, is_match = TRUE; i < file_len; i++)
+            is_match = is_match
+                       && (   chunk
+                           == *(const apr_uintptr_t *)
+                                    (file_for_suffix[i].curp + 1
+                                       - sizeof(apr_uintptr_t)));
 
+          if (! is_match)
+            break;
+
+          for (i = 0; i < file_len; i++)
+            {
+              file_for_suffix[i].curp -= sizeof(apr_uintptr_t);
+              can_read_word = can_read_word
+                              && (   (file_for_suffix[i].curp
+                                        - sizeof(apr_uintptr_t))
+                                  >= min_curp[i]);
+            }
+
+          /* We skipped 4 bytes, so there are no closing EOLs */
+          had_nl = FALSE;
+          had_cr = FALSE;
+        }
 #endif
 
       reached_prefix = file_for_suffix[0].chunk == suffix_min_chunk0