| Submitter | Augie Fackler |
|---|---|
| Date | Jan. 23, 2015, 9:06 p.m. |
| Message ID | <3435cf143b791612214a.1422047198@arthedain.pit.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/7539/ |
| State | Accepted |
| Commit | fefa5f2a1730acac2300e1af756cc913270eb0ca |
| Headers | show |
Comments
Nice finds! On Fri Jan 23 2015 at 1:08:40 PM Augie Fackler <raf@durin42.com> wrote: > # HG changeset patch > # User Augie Fackler <augie@google.com> > # Date 1422044344 18000 > # Fri Jan 23 15:19:04 2015 -0500 > # Branch stable > # Node ID 3435cf143b791612214a871337e8549e7d5c26a6 > # Parent de519517f597abd733961a8c549074eecb6c0ab3 > parsers: fix leak of err when asciilower hits a unicode decode error > > This is one of many errors detected in parsers.c by cpychecker[1]. I > haven't gone through all of them yet. > > 1: https://gcc-python-plugin.readthedocs.org/en/latest/index.html > > diff --git a/mercurial/parsers.c b/mercurial/parsers.c > --- a/mercurial/parsers.c > +++ b/mercurial/parsers.c > @@ -45,11 +45,11 @@ static char lowertable[128] = { > '\x30', '\x31', '\x32', '\x33', '\x34', '\x35', '\x36', '\x37', > '\x38', '\x39', '\x3a', '\x3b', '\x3c', '\x3d', '\x3e', '\x3f', > '\x40', > - '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', /* > A-G */ > + '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', /* > A-G */ > '\x68', '\x69', '\x6a', '\x6b', '\x6c', '\x6d', '\x6e', '\x6f', /* > H-O */ > '\x70', '\x71', '\x72', '\x73', '\x74', '\x75', '\x76', '\x77', /* > P-W */ > '\x78', '\x79', '\x7a', /* > X-Z */ > - '\x5b', '\x5c', '\x5d', '\x5e', '\x5f', > + '\x5b', '\x5c', '\x5d', '\x5e', '\x5f', > Were these changes intentional? (Replacing spaces by tabs? I'm not sure if it's just Inbox that presents tabs as spaces. Too lazy to check.) > '\x60', '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', > '\x68', '\x69', '\x6a', '\x6b', '\x6c', '\x6d', '\x6e', '\x6f', > '\x70', '\x71', '\x72', '\x73', '\x74', '\x75', '\x76', '\x77', > @@ -115,6 +115,7 @@ static PyObject *asciilower(PyObject *se > "ascii", str, len, i, (i + 1), > "unexpected code byte"); > PyErr_SetObject(PyExc_UnicodeDecodeError, err); > + Py_XDECREF(err); > Could be Py_DECREF instead? It seems like err should never be NULL. > goto quit; > } > newstr[i] = lowertable[(unsigned char)c]; > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel >
On Jan 23, 2015, at 4:21 PM, Martin von Zweigbergk <martinvonz@google.com> wrote: > Nice finds! > > On Fri Jan 23 2015 at 1:08:40 PM Augie Fackler <raf@durin42.com> wrote: > # HG changeset patch > # User Augie Fackler <augie@google.com> > # Date 1422044344 18000 > # Fri Jan 23 15:19:04 2015 -0500 > # Branch stable > # Node ID 3435cf143b791612214a871337e8549e7d5c26a6 > # Parent de519517f597abd733961a8c549074eecb6c0ab3 > parsers: fix leak of err when asciilower hits a unicode decode error > > This is one of many errors detected in parsers.c by cpychecker[1]. I > haven't gone through all of them yet. > > 1: https://gcc-python-plugin.readthedocs.org/en/latest/index.html > > diff --git a/mercurial/parsers.c b/mercurial/parsers.c > --- a/mercurial/parsers.c > +++ b/mercurial/parsers.c > @@ -45,11 +45,11 @@ static char lowertable[128] = { > '\x30', '\x31', '\x32', '\x33', '\x34', '\x35', '\x36', '\x37', > '\x38', '\x39', '\x3a', '\x3b', '\x3c', '\x3d', '\x3e', '\x3f', > '\x40', > - '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', /* A-G */ > + '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', /* A-G */ > '\x68', '\x69', '\x6a', '\x6b', '\x6c', '\x6d', '\x6e', '\x6f', /* H-O */ > '\x70', '\x71', '\x72', '\x73', '\x74', '\x75', '\x76', '\x77', /* P-W */ > '\x78', '\x79', '\x7a', /* X-Z */ > - '\x5b', '\x5c', '\x5d', '\x5e', '\x5f', > + '\x5b', '\x5c', '\x5d', '\x5e', '\x5f', > > Were these changes intentional? (Replacing spaces by tabs? I'm not sure if it's just Inbox that presents tabs as spaces. Too lazy to check.) No, that was emacs being jerk and "fixing" it for me. I can do a resend, or just drop this hunk. > > '\x60', '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', > '\x68', '\x69', '\x6a', '\x6b', '\x6c', '\x6d', '\x6e', '\x6f', > '\x70', '\x71', '\x72', '\x73', '\x74', '\x75', '\x76', '\x77', > @@ -115,6 +115,7 @@ static PyObject *asciilower(PyObject *se > "ascii", str, len, i, (i + 1), > "unexpected code byte"); > PyErr_SetObject(PyExc_UnicodeDecodeError, err); > + Py_XDECREF(err); > > Could be Py_DECREF instead? It seems like err should never be NULL. > > goto quit; > } > newstr[i] = lowertable[(unsigned char)c]; > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler writes: > # HG changeset patch > # User Augie Fackler <augie@google.com> > # Date 1422044344 18000 > # Fri Jan 23 15:19:04 2015 -0500 > # Branch stable > # Node ID 3435cf143b791612214a871337e8549e7d5c26a6 > # Parent de519517f597abd733961a8c549074eecb6c0ab3 > parsers: fix leak of err when asciilower hits a unicode decode error > > This is one of many errors detected in parsers.c by cpychecker[1]. I > haven't gone through all of them yet. > > 1: https://gcc-python-plugin.readthedocs.org/en/latest/index.html > > diff --git a/mercurial/parsers.c b/mercurial/parsers.c > --- a/mercurial/parsers.c > +++ b/mercurial/parsers.c > @@ -45,11 +45,11 @@ static char lowertable[128] = { > '\x30', '\x31', '\x32', '\x33', '\x34', '\x35', '\x36', '\x37', > '\x38', '\x39', '\x3a', '\x3b', '\x3c', '\x3d', '\x3e', '\x3f', > '\x40', > - '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', /* A-G */ > + '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', /* A-G */ What kind of whitespace mode are you using that reindents these bits incorrectly? > '\x68', '\x69', '\x6a', '\x6b', '\x6c', '\x6d', '\x6e', '\x6f', /* H-O */ > '\x70', '\x71', '\x72', '\x73', '\x74', '\x75', '\x76', '\x77', /* P-W */ > '\x78', '\x79', '\x7a', /* X-Z */ > - '\x5b', '\x5c', '\x5d', '\x5e', '\x5f', > + '\x5b', '\x5c', '\x5d', '\x5e', '\x5f', Same question here.
Patch
diff --git a/mercurial/parsers.c b/mercurial/parsers.c --- a/mercurial/parsers.c +++ b/mercurial/parsers.c @@ -45,11 +45,11 @@ static char lowertable[128] = { '\x30', '\x31', '\x32', '\x33', '\x34', '\x35', '\x36', '\x37', '\x38', '\x39', '\x3a', '\x3b', '\x3c', '\x3d', '\x3e', '\x3f', '\x40', - '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', /* A-G */ + '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', /* A-G */ '\x68', '\x69', '\x6a', '\x6b', '\x6c', '\x6d', '\x6e', '\x6f', /* H-O */ '\x70', '\x71', '\x72', '\x73', '\x74', '\x75', '\x76', '\x77', /* P-W */ '\x78', '\x79', '\x7a', /* X-Z */ - '\x5b', '\x5c', '\x5d', '\x5e', '\x5f', + '\x5b', '\x5c', '\x5d', '\x5e', '\x5f', '\x60', '\x61', '\x62', '\x63', '\x64', '\x65', '\x66', '\x67', '\x68', '\x69', '\x6a', '\x6b', '\x6c', '\x6d', '\x6e', '\x6f', '\x70', '\x71', '\x72', '\x73', '\x74', '\x75', '\x76', '\x77', @@ -115,6 +115,7 @@ static PyObject *asciilower(PyObject *se "ascii", str, len, i, (i + 1), "unexpected code byte"); PyErr_SetObject(PyExc_UnicodeDecodeError, err); + Py_XDECREF(err); goto quit; } newstr[i] = lowertable[(unsigned char)c];