Author loewis
Recipients amaury.forgeotdarc, bob.ippolito, georg.brandl, loewis, vstinner
Date 2009-01-04.13:22:30
SpamBayes Score 0.000123209
Marked as misclassified No
Message-id <000e0cd6abb2df64c2045fa810dc@google.com>
In-reply-to
Content
http://codereview.appspot.com/7311/diff/1/8
File Lib/json/decoder.py (right):

http://codereview.appspot.com/7311/diff/1/8#newcode55
Line 55: def py_scanstring(s, end, encoding=None, strict=True,
_b=BACKSLASH, _m=STRINGCHUNK.match):
This function should get some comments what all the various cases are
(preferably speaking with the terms of JSON spec, i.e. chars, char, ...)

http://codereview.appspot.com/7311/diff/1/8#newcode71
Line 71: _append(content)
# 3 cases: end of string, control character, escape sequence

http://codereview.appspot.com/7311/diff/1/8#newcode76
Line 76: msg = "Invalid control character {0!r} at".format(esc)
esc isn't assigned until a few lines later. Is this really correct?

http://codereview.appspot.com/7311/diff/1/8#newcode104
Line 104: raise ValueError
No message?

http://codereview.appspot.com/7311/diff/1/8#newcode107
Line 107: raise ValueError
No message?

http://codereview.appspot.com/7311/diff/1/8#newcode111
Line 111: m = unichr(uni)
What's the purpose of m?

http://codereview.appspot.com/7311/diff/1/8#newcode127
Line 127: nextchar = s[end:end + 1]
Why not s[end]? Add comment if this is necessary.

http://codereview.appspot.com/7311/diff/1/8#newcode132
Line 132: nextchar = s[end:end + 1]
Likewise. There are more places where it does slicing, but also places
where it does indexing, in this function.

http://codereview.appspot.com/7311/diff/1/8#newcode290
Line 290: following strings: -Infinity, Infinity, NaN.
This sounds like an incompatible change.

http://codereview.appspot.com/7311/diff/1/8#newcode317
Line 317: def raw_decode(self, s, idx=0):
That looks like an incompatible change

http://codereview.appspot.com/7311/diff/1/9
File Modules/_json.c (right):

http://codereview.appspot.com/7311/diff/1/9#newcode196
Line 196: output_size *= 2;
You might want to check for integer overflow here.

http://codereview.appspot.com/7311/diff/1/9#newcode215
Line 215: ascii_escape_str(PyObject *pystr)
Please attach a comment to each function, telling what the function
does.

http://codereview.appspot.com/7311/diff/1/9#newcode733
Line 733: "..."
Some text should probably be added here.

http://codereview.appspot.com/7311/diff/1/9#newcode1320
Line 1320: if ((idx + 3 < length) && str[idx + 1] == 'u' && str[idx + 2]
== 'l' && str[idx + 3] == 'l') {
Is this really faster than a strncmp?

http://codereview.appspot.com/7311/diff/1/9#newcode1528
Line 1528: PyTypeObject PyScannerType = {
I think scanner objects should participate in cyclic gc.

http://codereview.appspot.com/7311/diff/1/9#newcode2025
Line 2025: "make_encoder",       /* tp_name */
That is a confusing type name. How about "Encoder"?

http://codereview.appspot.com/7311
History
Date User Action Args
2009-01-04 13:22:32loewissetrecipients: + loewis, georg.brandl, bob.ippolito, amaury.forgeotdarc, vstinner
2009-01-04 13:22:32loewislinkissue4136 messages
2009-01-04 13:22:30loewiscreate