From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
Cc: | pgsql-committers(at)postgresql(dot)org |
Subject: | Re: pgsql: PL/Python: Fix potential NULL pointer dereference |
Date: | 2017-12-05 19:45:59 |
Message-ID: | 16695.1512503159@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
> On 11/28/17 13:35, Peter Eisentraut wrote:
>> OK, I'll revert and rethink.
> How about this instead?
This looks better, but I think there are a couple more things:
* Seems like you ought to s/Py_DECREF(result)/Py_XDECREF(result)/ in the
PG_CATCH block (line 451 in HEAD). As this is coded, although result
is set to NULL within the PG_TRY, it doesn't seem possible for control
to reach PG_CATCH after that ... but that's a pretty fragile assumption.
* You'd also need to declare "result" as
PLyResultObject *volatile result;
if you intend to change it within the PG_TRY block.
On the whole it seems like it might be better to dodge this whole business
of changing "result" inside the TRY. You could do that if you did
something like
result->rows = PyList_New(rows);
- if (!result->rows)
- {
- Py_DECREF(result);
- result = NULL;
- }
- else
+ if (result->rows)
{
PLy_input_setup_tuple(&ininfo, tuptable->tupdesc,
and then add after the PG_END_TRY
if (!result->rows)
{
Py_DECREF(result);
result = NULL;
}
The repeated test is a bit annoying but maybe it's better than the
alternative. You'd probably need a comment explaining why it's
like that, though.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2017-12-05 20:05:44 | Re: pgsql: Add some regression tests that exercise hash join code. |
Previous Message | Robert Haas | 2017-12-05 19:39:29 | pgsql: Fix accumulation of parallel worker instrumentation. |