Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3073)

#27587: Issues, reported by PVS-Studio static analyzer

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 8 months ago by belikov
Modified:
2 years, 7 months ago
Reviewers:
victor.stinner, rosuav, berker.peksag
CC:
pmoore, haypo, christian.heimes, tim.golden, cvrebert, skrah, devnull_psf.upfronthosting.co.za, Rosuav, berkerpeksag, Martin Panter, Zach Ware, steve.dower, belikov_viva64.com
Visibility:
Public.

Patch Set 1 #

Total comments: 1

Patch Set 2 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Python/pystate.c View 1 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 3
victor.stinner_gmail.com
https://bugs.python.org/review/27587/diff/17968/Python/pystate.c File Python/pystate.c (right): https://bugs.python.org/review/27587/diff/17968/Python/pystate.c#newcode286 Python/pystate.c:286: if (!def) Can you add an assert(PyErr_Occurred()) to ensure ...
2 years, 8 months ago #1
Rosuav
On 2016/07/25 14:31:59, haypo wrote: > https://bugs.python.org/review/27587/diff/17968/Python/pystate.c > File Python/pystate.c (right): > > https://bugs.python.org/review/27587/diff/17968/Python/pystate.c#newcode286 > ...
2 years, 8 months ago #2
berkerpeksag
2 years, 7 months ago #3
On 2016/07/25 14:31:59, haypo wrote:
> https://bugs.python.org/review/27587/diff/17968/Python/pystate.c
> File Python/pystate.c (right):
> 
> https://bugs.python.org/review/27587/diff/17968/Python/pystate.c#newcode286
> Python/pystate.c:286: if (!def)
> Can you add an assert(PyErr_Occurred()) to ensure that an exception is already
> raised? You must also add { ... } around the if body.

Added the assert in issue27587_pystate_addmodule_v2.diff. Py_FatalError is
raised by PyState_AddModule:

int
PyState_AddModule(PyObject* module, struct PyModuleDef* def)
{
    ...
    if (!def) {
        Py_FatalError("PyState_AddModule: Module Definition is NULL");
        return -1;
    }
    ...

I'm not sure if the assert statement is needed in this case.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+