Rev 3077: Enable some error checking, and small amount of code cleanup. in http://bzr.arbash-meinel.com/branches/bzr/0.93-dev/patience_tuples

[John Arbash Meinel]

At http://bzr.arbash-meinel.com/branches/bzr/0.93-dev/patience_tuples

------------------------------------------------------------
revno: 3077
revision-id:john at arbash-meinel.com-20071204162828-chfnl5ylzkh0y2ll
parent: john at arbash-meinel.com-20071204160709-mmzmchn2daf0quma
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: patience_tuples
timestamp: Tue 2007-12-04 10:28:28 -0600
message:
  Enable some error checking, and small amount of code cleanup.
  If PyObject_Hash() or PyObject_Length() fails, we should raise an exception, not
  silently swallow it.
modified:
  bzrlib/_patiencediff_c.c       _patiencediff_c.c-20070721205602-q3imkipwlgagp3cy-1
  bzrlib/tests/test_diff.py      testdiff.py-20050727164403-d1a3496ebb12e339
-------------- next part --------------
=== modified file 'bzrlib/_patiencediff_c.c'
--- a/bzrlib/_patiencediff_c.c	2007-12-04 16:07:09 +0000
+++ b/bzrlib/_patiencediff_c.c	2007-12-04 16:28:28 +0000
@@ -546,12 +546,9 @@
 static Py_ssize_t
 load_lines(PyObject *orig, struct line **lines)
 {
-    Py_ssize_t size, i, j, tuple_len;
-    Py_ssize_t total_len;
-    // int h;
-    // char *p;
+    Py_ssize_t size, i, j, tuple_len, cur_len, total_len;
     struct line *line;
-    PyObject *seq, *item;
+    PyObject *seq, *item, *subitem;
 
     seq = PySequence_Fast(orig, "sequence expected");
     if (seq == NULL) {
@@ -574,23 +571,39 @@
     for (i = 0; i < size; i++) {
         item = PySequence_Fast_GET_ITEM(seq, i);
         if (PyString_Check(item)) {
-            // we could use the 'djb2' hash here if we find it is better than
-            // PyObject_Hash, however it seems a lot slower to compute, and
-            // doesn't give particularly better results
+            /* we could use the 'djb2' hash here if we find it is better than
+               PyObject_Hash, however it seems measurably slower to compute,
+               and doesn't give particularly better results
+             */
             line->len = PyString_GET_SIZE(item);
         } else if (PyTuple_Check(item)) {
             total_len = 0;
             tuple_len = PyObject_Length(item);
             for (j = 0; j < tuple_len; ++j) {
-                total_len += PyObject_Length(PySequence_Fast_GET_ITEM(item, j));
+                subitem = PySequence_Fast_GET_ITEM(item, j);
+                cur_len = PyObject_Length(subitem);
+                if (cur_len == -1) {
+                    /* Error */
+                    return -1;
+                }
+                total_len += cur_len;
             }
             line->len = total_len;
         } else {
-            // Generic length
-            line->len = PyObject_Length(item);
+            /* Generic length */
+            cur_len = PyObject_Length(item);
+            if (cur_len == -1) {
+                /* Error */
+                return -1;
+            }
+            line->len = cur_len;
         }
         line->data = item;
         line->hash = PyObject_Hash(item);
+        if (line->hash == (-1)) {
+            /* Propogate the hash exception */
+            return -1;
+        }
         line->next = SENTINEL;
         line++;
     }

=== modified file 'bzrlib/tests/test_diff.py'
--- a/bzrlib/tests/test_diff.py	2007-12-04 15:24:39 +0000
+++ b/bzrlib/tests/test_diff.py	2007-12-04 16:28:28 +0000
@@ -1079,6 +1079,17 @@
         self._PatienceSequenceMatcher = \
             bzrlib._patiencediff_c.PatienceSequenceMatcher_c
 
+    def test_unhashable(self):
+        """We should get a proper exception here."""
+        # A sub-list is unhashable
+        e = self.assertRaises(TypeError, self._PatienceSequenceMatcher,
+                                         None, [[]], [])
+
+    def test_no_length(self):
+        # Objects have no length
+        e = self.assertRaises(TypeError, self._PatienceSequenceMatcher,
+                                         None, [object()], [])
+
 
 class TestPatienceDiffLibFiles(TestCaseInTempDir):