Merge lp:~jameinel/bzr/2.0.1-btree-better-errors into lp:bzr/2.0

Proposed by John A Meinel
Status: Merged
Merged at revision: not available
Proposed branch: lp:~jameinel/bzr/2.0.1-btree-better-errors
Merge into: lp:bzr/2.0
Diff against target: 63 lines
1 file modified
bzrlib/_btree_serializer_pyx.pyx (+6/-8)
To merge this branch: bzr merge lp:~jameinel/bzr/2.0.1-btree-better-errors
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Review via email: mp+12759@code.launchpad.net
To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

In some of my other work that I've been doing I ended up feeding some bogus data to the btree serializer.

In the 'process_line()' function, we declare that returning -1 indicates an exception is set, however there are several code paths that return -1 during an error without raising an exception. In those cases I end up with:
Traceback (most recent call last):
  File "<string>", line 1, in <module>
SystemError: error return without exception set

Rather than getting a nice parse error sort of condition.

So I fixed all of those to be nicer. I figure this is trivial enough to go into 2.0.1 (pre-emptive bug fix :).

Revision history for this message
Robert Collins (lifeless) wrote :

 review +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/_btree_serializer_pyx.pyx'
2--- bzrlib/_btree_serializer_pyx.pyx 2009-06-22 12:52:39 +0000
3+++ bzrlib/_btree_serializer_pyx.pyx 2009-10-01 20:55:19 +0000
4@@ -186,14 +186,13 @@
5 # And the next string is right after it
6 self._cur_str = last + 1
7 # The last character is right before the '\n'
8- last = last
9
10 if last == self._start:
11 # parsed it all.
12 return 0
13 if last < self._start:
14 # Unexpected error condition - fail
15- return -1
16+ raise AssertionError("last < self._start")
17 if 0 == self._header_found:
18 # The first line in a leaf node is the header "type=leaf\n"
19 if strncmp("type=leaf", self._start, last - self._start) == 0:
20@@ -202,14 +201,13 @@
21 else:
22 raise AssertionError('Node did not start with "type=leaf": %r'
23 % (safe_string_from_size(self._start, last - self._start)))
24- return -1
25
26 key = self.extract_key(last)
27 # find the value area
28 temp_ptr = <char*>_my_memrchr(self._start, c'\0', last - self._start)
29 if temp_ptr == NULL:
30 # Invalid line
31- return -1
32+ raise AssertionError("Failed to find the value area")
33 else:
34 # capture the value string
35 value = safe_string_from_size(temp_ptr + 1, last - temp_ptr - 1)
36@@ -223,15 +221,15 @@
37 # extract a reference list
38 loop_counter = loop_counter + 1
39 if last < self._start:
40- return -1
41+ raise AssertionError("last < self._start")
42 # find the next reference list end point:
43 temp_ptr = <char*>memchr(self._start, c'\t', last - self._start)
44 if temp_ptr == NULL:
45 # Only valid for the last list
46 if loop_counter != self.ref_list_length:
47 # Invalid line
48- return -1
49- raise AssertionError("invalid key")
50+ raise AssertionError(
51+ "invalid key, loop_counter != self.ref_list_length")
52 else:
53 # scan to the end of the ref list area
54 ref_ptr = last
55@@ -257,7 +255,7 @@
56 else:
57 if last != self._start:
58 # unexpected reference data present
59- return -1
60+ raise AssertionError("unexpected reference data present")
61 node_value = (value, ())
62 PyList_Append(self.keys, (key, node_value))
63 return 0

Subscribers

People subscribed via source and target branches