PATCH: [ 1384181 ] Fix byte difference fencepost/infinite loop

  Src: CompareOptions.h ConfigLog.cpp DiffWrapper.cpp DiffWrapper.h
   Merge.dsp PatchDlg.cpp stringdiffs.cpp

diff --git a/Src/Changes.txt b/Src/Changes.txt
index 33ca678..c040523 100644 (file)
--- a/Src/Changes.txt
+++ b/Src/Changes.txt
@@ -2,6 +2,9 @@
  PATCH: [ 1383120 ] Fix background colors in WinMerge
   Src: SyntaxColors.cpp SyntaxColors.h
   Src/Common: OptionsMgr.cpp OptionsMgr.h
+ PATCH: [ 1384181 ] Fix byte difference fencepost/infinite loop
+  Src: CompareOptions.h ConfigLog.cpp DiffWrapper.cpp DiffWrapper.h
+   Merge.dsp PatchDlg.cpp stringdiffs.cpp
 
 2005-12-17 Kimmo
  BUG: [ 1379277 ] typo fix for docs
@@ -131,7 +134,6 @@
   Src\Common: RegKey.cpp
   NEW Src\FileLocation.h
 
-
 2005-12-09 Perry
  PATCH: [ 1188787 ] New Chinese translation rc file (from PATCH 1183476)
   Remainder of dialogs

diff --git a/Src/CompareOptions.h b/Src/CompareOptions.h
new file mode 100644 (file)
index 0000000..78db1ff
--- /dev/null
+++ b/Src/CompareOptions.h
@@ -0,0 +1,23 @@
+#ifndef CompareOptions_h_included
+#define CompareOptions_h_included
+
+/**
+ * @brief Whether to ignore whitespace (or to ignore changes in whitespace)
+ *
+ * Examples:
+ * "abc def" is only equivalent to "abcdef" under WHITESPACE_IGNORE_ALL
+ *
+ * but "abc def" is equivalent to "abc    def" under both 
+ *   WHITESPACE_IGNORE_CHANGE and WHITESPACE_IGNORE_ALL
+ *
+ * Also, trailing and leading whitespace is ignored for both
+ *   WHITESPACE_IGNORE_CHANGE and WHITESPACE_IGNORE_ALL
+ */
+enum
+{
+       WHITESPACE_COMPARE_ALL = 0,    /**< no special handling of whitespace */
+       WHITESPACE_IGNORE_CHANGE,      /**< ignore changes in whitespace */
+       WHITESPACE_IGNORE_ALL          /**< ignore whitespace altogether */
+};
+
+#endif // CompareOptions_h_included

diff --git a/Src/ConfigLog.cpp b/Src/ConfigLog.cpp
index 6c2bc1d..d4bcedf 100644 (file)
--- a/Src/ConfigLog.cpp
+++ b/Src/ConfigLog.cpp
@@ -36,6 +36,7 @@
 #include "unicoder.h"
 #include "codepage.h"
 #include "7zcommon.h"
+#include "CompareOptions.h"
 
 
 // Static function declarations

diff --git a/Src/DiffWrapper.cpp b/Src/DiffWrapper.cpp
index 123c8bc..abb0a4d 100644 (file)
--- a/Src/DiffWrapper.cpp
+++ b/Src/DiffWrapper.cpp
@@ -39,6 +39,7 @@
 #include "codepage_detect.h"
 #include "paths.h"
 #include "IAbortable.h"
+#include "CompareOptions.h"
 
 #ifdef _DEBUG
 #define new DEBUG_NEW

diff --git a/Src/DiffWrapper.h b/Src/DiffWrapper.h
index 2d1383e..7c49baa 100644 (file)
--- a/Src/DiffWrapper.h
+++ b/Src/DiffWrapper.h
@@ -47,18 +47,6 @@ enum
 };
 
 /**
- * @brief Whitespace options
- *
- * DIFFOPTIONS structs nIgnoreWhitespace can have these values
- */
-enum
-{
-       WHITESPACE_COMPARE_ALL = 0,
-       WHITESPACE_IGNORE_CHANGE,
-       WHITESPACE_IGNORE_ALL
-};
-
-/**
  * @brief Patch styles
  *
  * Diffutils can output patch in these formats.

diff --git a/Src/Merge.dsp b/Src/Merge.dsp
index 16274a3..18dbc8a 100644 (file)
--- a/Src/Merge.dsp
+++ b/Src/Merge.dsp
@@ -925,6 +925,10 @@ SOURCE=.\ColorButton.h
 # End Source File
 # Begin Source File
 
+SOURCE=.\CompareOptions.h
+# End Source File
+# Begin Source File
+
 SOURCE=.\CompareStatisticsDlg.h
 # End Source File
 # Begin Source File

diff --git a/Src/PatchDlg.cpp b/Src/PatchDlg.cpp
index 61c8ceb..2028b75 100644 (file)
--- a/Src/PatchDlg.cpp
+++ b/Src/PatchDlg.cpp
@@ -28,6 +28,7 @@
 #include "diff.h"
 #include "coretools.h"
 #include "paths.h"
+#include "CompareOptions.h"
 
 #ifdef _DEBUG
 #define new DEBUG_NEW

diff --git a/Src/stringdiffs.cpp b/Src/stringdiffs.cpp
index 0658c1d..841f557 100644 (file)
--- a/Src/stringdiffs.cpp
+++ b/Src/stringdiffs.cpp
@@ -10,10 +10,7 @@
 #include "stdafx.h"
 #include <mbctype.h>
 #include "stringdiffs.h"
-
-class stuff {
-       int j;
-};
+#include "CompareOptions.h"
 #include "stringdiffsi.h"
 
 #ifdef _DEBUG
@@ -519,7 +516,38 @@ LastChar(LPCTSTR psz, int len)
 }
 
 /**
+ * @brief advance current pointer over whitespace, until not whitespace or beyond end
+ * @param pcurrent [in,out] current location (to be advanced)
+ * @param end [in] last valid position (only go one beyond this)
+ */
+static void
+AdvanceOverWhitespace(LPCTSTR * pcurrent, LPCTSTR end)
+{
+       // advance over whitespace
+       while (*pcurrent <= end && isSafeWhitespace(**pcurrent))
+               ++(*pcurrent); // DBCS safe because of isSafeWhitespace above
+}
+
+/**
+ * @brief back current pointer over whitespace, until not whitespace or at start
+ * @param pcurrent [in,out] current location (to be backed up)
+ * @param start [in] first valid position (do not go before this)
+ *
+ * NB: Unlike AdvanceOverWhitespace, this will not go over the start
+ * This because WinMerge doesn't need to, and also CharPrev cannot easily do so
+ */
+static void
+RetreatOverWhitespace(LPCTSTR * pcurrent, LPCTSTR start)
+{
+       // back over whitespace
+       while (*pcurrent > start && isSafeWhitespace(**pcurrent))
+               *pcurrent = CharPrev(start, *pcurrent); // DBCS safe because of isSafeWhitespace above
+}
+
+/**
  * @brief Compute begin1,begin2,end1,end2 to display byte difference between strings str1 & str2
+ * @param casitive [in] true for case-sensitive, false for case-insensitive
+ * @param xwhite [in] This governs whether we handle whitespace specially (see WHITESPACE_COMPARE_ALL, WHITESPACE_IGNORE_CHANGE, WHITESPACE_IGNORE_ALL)
  *
  * Assumes whitespace is never leadbyte or trailbyte!
  */
@@ -558,8 +586,11 @@ sd_ComputeByteDiff(CString & str1, CString & str2,
        LPCTSTR pen1 = LastChar(py1, len1);
        LPCTSTR pen2 = LastChar(py2, len2);
 
-       if (xwhite)
+       if (xwhite != WHITESPACE_COMPARE_ALL)
        {
+               // Ignore leading and trailing whitespace
+               // by advancing py1 and py2
+               // and retreating pen1 and pen2
                while (py1 < pen1 && isSafeWhitespace(*py1))
                        ++py1; // DBCS safe because of isSafeWhitespace above
                while (py2 < pen2 && isSafeWhitespace(*py2))
@@ -599,24 +630,33 @@ sd_ComputeByteDiff(CString & str1, CString & str2,
                }
 
                // handle all the whitespace logic (due to WinMerge whitespace settings)
-               if (xwhite && isSafeWhitespace(*py1))
+               if (xwhite!=WHITESPACE_COMPARE_ALL && isSafeWhitespace(*py1))
                {
-                       if (xwhite==1 && !isSafeWhitespace(*py2))
+                       if (xwhite==WHITESPACE_IGNORE_CHANGE && !isSafeWhitespace(*py2))
+                       {
+                               // py1 is white but py2 is not
+                               // in WHITESPACE_IGNORE_CHANGE mode,
+                               // this doesn't qualify as skippable whitespace
                                break; // done with forward search
+                       }
                        // gobble up all whitespace in current area
-                       while (isSafeWhitespace(*py1))
-                               ++py1; // DBCS safe because of isSafeWhitespace above
-                       while (isSafeWhitespace(*py2))
-                               ++py2; // DBCS safe because of isSafeWhitespace above
+                       AdvanceOverWhitespace(&py1, pen1); // will go beyond end
+                       AdvanceOverWhitespace(&py2, pen2); // will go beyond end
                        continue;
 
                }
-               if (xwhite && isSafeWhitespace(*py2))
+               if (xwhite!=WHITESPACE_COMPARE_ALL && isSafeWhitespace(*py2))
                {
-                       if (xwhite==1)
+                       if (xwhite==WHITESPACE_IGNORE_CHANGE && !isSafeWhitespace(*py1))
+                       {
+                               // py2 is white but py1 is not
+                               // in WHITESPACE_IGNORE_CHANGE mode,
+                               // this doesn't qualify as skippable whitespace
                                break; // done with forward search
-                       while (py2 < pen2 && isSafeWhitespace(*py2))
-                               ++py2; // multibyte safe because of isSafeWhitespace above
+                       }
+                       // gobble up all whitespace in current area
+                       AdvanceOverWhitespace(&py1, pen1); // will go beyond end
+                       AdvanceOverWhitespace(&py2, pen2); // will go beyond end
                        continue;
                }
 
@@ -677,24 +717,33 @@ sd_ComputeByteDiff(CString & str1, CString & str2,
                        }
 
                        // handle all the whitespace logic (due to WinMerge whitespace settings)
-                       if (xwhite && isSafeWhitespace(*pz1))
+                       if (xwhite!=WHITESPACE_COMPARE_ALL && isSafeWhitespace(*pz1))
                        {
-                               if (xwhite==1 && !isSafeWhitespace(*pz2))
+                               if (xwhite==WHITESPACE_IGNORE_CHANGE && !isSafeWhitespace(*pz2))
+                               {
+                                       // pz1 is white but pz2 is not
+                                       // in WHITESPACE_IGNORE_CHANGE mode,
+                                       // this doesn't qualify as skippable whitespace
                                        break; // done with reverse search
+                               }
                                // gobble up all whitespace in current area
-                               while (pz1 > py1 && isSafeWhitespace(*pz1))
-                                       pz1 = CharPrev(py1, pz1);
-                               while (pz2 > py2 && isSafeWhitespace(*pz2))
-                                       pz2 = CharPrev(py2, pz2);
+                               RetreatOverWhitespace(&pz1, py1); // will not go over beginning
+                               RetreatOverWhitespace(&pz2, py2); // will not go over beginning
                                continue;
 
                        }
-                       if (xwhite && isSafeWhitespace(*pz2))
+                       if (xwhite!=WHITESPACE_COMPARE_ALL && isSafeWhitespace(*pz2))
                        {
-                               if (xwhite==1)
+                               if (xwhite==WHITESPACE_IGNORE_CHANGE && !isSafeWhitespace(*pz1))
+                               {
+                                       // pz2 is white but pz1 is not
+                                       // in WHITESPACE_IGNORE_CHANGE mode,
+                                       // this doesn't qualify as skippable whitespace
                                        break; // done with reverse search
-                               while (pz2 > py2 && isSafeWhitespace(*pz2))
-                                       pz2 = CharPrev(py2, pz2);
+                               }
+                               // gobble up all whitespace in current area
+                               RetreatOverWhitespace(&pz1, py1); // will not go over beginning
+                               RetreatOverWhitespace(&pz2, py2); // will not go over beginning
                                continue;
                        }