On 18/02/12 21:18, Jan Urbański wrote:
> On 18/02/12 21:17, Tom Lane wrote:
>> =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= <wulc...@wulczer.org> writes:
>>> On 18/02/12 20:30, Tom Lane wrote:
>>>> Dave Malcolm at Red Hat has been working on a static code analysis tool
>>>> for Python-related C code. He reports here on some preliminary results
>>>> for plpython.c:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=795011
>>
>> If you find any live bugs, it'd likely be better to deal with them as
>> a separate patch so that we can back-patch ...
>
> Sure, I meant to say I'll look at these as well, but will make them into
> a separate patch.
Here's a patch that fixes everything I was sure was an actual bug. The
rest of the warnings seem to be caused by the tool not knowing that
elog(ERROR) throws a longjmp and things like "we never unref this
object, so it can't disappear mid-execution".
Attached are patches for HEAD and for 9.1.x (since splitting plpython.c
in 9.2 was kind of my idea I felt bad about you having to back-patch
this so I tried to do the necessary legwork myself; I hope the attached
is what you need).
BTW, that tool is quite handy, I'll have to try running it over psycopg2.
Cheers,
Jan
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 530b5f0..2b064c5 100644
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
*************** PLyList_FromArray(PLyDatumToOb *arg, Dat
*** 2345,2350 ****
--- 2345,2352 ----
length = ARR_DIMS(array)[0];
lbound = ARR_LBOUND(array)[0];
list = PyList_New(length);
+ if (list == NULL)
+ elog(ERROR, "could not transform Python list to array");
for (i = 0; i < length; i++)
{
*************** PLy_spi_execute_query(char *query, long
*** 3664,3670 ****
int rv;
volatile MemoryContext oldcontext;
volatile ResourceOwner oldowner;
! PyObject *ret;
oldcontext = CurrentMemoryContext;
oldowner = CurrentResourceOwner;
--- 3666,3672 ----
int rv;
volatile MemoryContext oldcontext;
volatile ResourceOwner oldowner;
! PyObject *ret = NULL;
oldcontext = CurrentMemoryContext;
oldowner = CurrentResourceOwner;
*************** PLy_spi_execute_query(char *query, long
*** 3727,3732 ****
--- 3729,3735 ----
if (rv < 0)
{
+ Py_XDECREF(ret);
PLy_exception_set(PLy_exc_spi_error,
"SPI_execute failed: %s",
SPI_result_code_string(rv));
*************** PLy_generate_spi_exceptions(PyObject *mo
*** 3967,3973 ****
--- 3970,3982 ----
PyObject *sqlstate;
PyObject *dict = PyDict_New();
+ if (dict == NULL)
+ elog(ERROR, "could not generate SPI exceptions");
+
sqlstate = PyString_FromString(unpack_sql_state(exception_map[i].sqlstate));
+ if (sqlstate == NULL)
+ elog(ERROR, "could not generate SPI exceptions");
+
PyDict_SetItemString(dict, "sqlstate", sqlstate);
Py_DECREF(sqlstate);
exc = PyErr_NewException(exception_map[i].name, base, dict);
*************** PLy_add_exceptions(PyObject *plpy)
*** 4008,4013 ****
--- 4017,4027 ----
PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL);
+ if (PLy_exc_error == NULL ||
+ PLy_exc_fatal == NULL ||
+ PLy_exc_spi_error == NULL)
+ elog(ERROR, "could not create the base SPI exceptions");
+
Py_INCREF(PLy_exc_error);
PyModule_AddObject(plpy, "Error", PLy_exc_error);
Py_INCREF(PLy_exc_fatal);
*************** PLy_init_interp(void)
*** 4124,4129 ****
--- 4138,4145 ----
Py_INCREF(mainmod);
PLy_interp_globals = PyModule_GetDict(mainmod);
PLy_interp_safe_globals = PyDict_New();
+ if (PLy_interp_safe_globals == NULL)
+ PLy_elog(ERROR, "could not create globals");
PyDict_SetItemString(PLy_interp_globals, "GD", PLy_interp_safe_globals);
Py_DECREF(mainmod);
if (PLy_interp_globals == NULL || PyErr_Occurred())
*************** PLy_init_plpy(void)
*** 4164,4169 ****
--- 4180,4187 ----
main_mod = PyImport_AddModule("__main__");
main_dict = PyModule_GetDict(main_mod);
plpy_mod = PyImport_AddModule("plpy");
+ if (plpy_mod == NULL)
+ elog(ERROR, "could not initialize plpy");
PyDict_SetItemString(main_dict, "plpy", plpy_mod);
if (PyErr_Occurred())
elog(ERROR, "could not initialize plpy");
*************** PLy_output(volatile int level, PyObject
*** 4231,4238 ****
* decoration.
*/
PyObject *o;
- PyArg_UnpackTuple(args, "plpy.elog", 1, 1, &o);
so = PyObject_Str(o);
}
else
--- 4249,4260 ----
* decoration.
*/
PyObject *o;
+ int result;
+
+ result = PyArg_UnpackTuple(args, "plpy.elog", 1, 1, &o);
+ if (!result)
+ elog(ERROR, "could not unpack arguments in plpy.elog");
so = PyObject_Str(o);
}
else
*************** get_source_line(const char *src, int lin
*** 4554,4559 ****
--- 4576,4585 ----
const char *next = src;
int current = 0;
+ /* sanity check */
+ if (lineno <= 0)
+ return NULL;
+
while (current < lineno)
{
s = next;
diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c
index 741980c..332898d 100644
*** a/src/pl/plpython/plpy_elog.c
--- b/src/pl/plpython/plpy_elog.c
*************** get_source_line(const char *src, int lin
*** 365,370 ****
--- 365,374 ----
const char *next = src;
int current = 0;
+ /* sanity check */
+ if (lineno <= 0)
+ return NULL;
+
while (current < lineno)
{
s = next;
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index ae9d87e..f259ecf 100644
*** a/src/pl/plpython/plpy_main.c
--- b/src/pl/plpython/plpy_main.c
*************** PLy_init_interp(void)
*** 133,138 ****
--- 133,140 ----
Py_INCREF(mainmod);
PLy_interp_globals = PyModule_GetDict(mainmod);
PLy_interp_safe_globals = PyDict_New();
+ if (PLy_interp_safe_globals == NULL)
+ PLy_elog(ERROR, "could not create globals");
PyDict_SetItemString(PLy_interp_globals, "GD", PLy_interp_safe_globals);
Py_DECREF(mainmod);
if (PLy_interp_globals == NULL || PyErr_Occurred())
diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c
index d2d0a2a..7f86434 100644
*** a/src/pl/plpython/plpy_plpymodule.c
--- b/src/pl/plpython/plpy_plpymodule.c
*************** PLy_init_plpy(void)
*** 173,178 ****
--- 173,180 ----
main_mod = PyImport_AddModule("__main__");
main_dict = PyModule_GetDict(main_mod);
plpy_mod = PyImport_AddModule("plpy");
+ if (plpy_mod == NULL)
+ elog(ERROR, "could not initialize plpy");
PyDict_SetItemString(main_dict, "plpy", plpy_mod);
if (PyErr_Occurred())
elog(ERROR, "could not initialize plpy");
*************** PLy_add_exceptions(PyObject *plpy)
*** 208,213 ****
--- 210,220 ----
PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL);
+ if (PLy_exc_error == NULL ||
+ PLy_exc_fatal == NULL ||
+ PLy_exc_spi_error == NULL)
+ elog(ERROR, "could not create the base SPI exceptions");
+
Py_INCREF(PLy_exc_error);
PyModule_AddObject(plpy, "Error", PLy_exc_error);
Py_INCREF(PLy_exc_fatal);
*************** PLy_generate_spi_exceptions(PyObject *mo
*** 241,247 ****
--- 248,260 ----
PyObject *sqlstate;
PyObject *dict = PyDict_New();
+ if (dict == NULL)
+ elog(ERROR, "could not generate SPI exceptions");
+
sqlstate = PyString_FromString(unpack_sql_state(exception_map[i].sqlstate));
+ if (sqlstate == NULL)
+ elog(ERROR, "could not generate SPI exceptions");
+
PyDict_SetItemString(dict, "sqlstate", sqlstate);
Py_DECREF(sqlstate);
exc = PyErr_NewException(exception_map[i].name, base, dict);
*************** PLy_output(volatile int level, PyObject
*** 369,376 ****
* decoration.
*/
PyObject *o;
- PyArg_UnpackTuple(args, "plpy.elog", 1, 1, &o);
so = PyObject_Str(o);
}
else
--- 382,393 ----
* decoration.
*/
PyObject *o;
+ int result;
+
+ result = PyArg_UnpackTuple(args, "plpy.elog", 1, 1, &o);
+ if (!result)
+ elog(ERROR, "could not unpack arguments in plpy.elog");
so = PyObject_Str(o);
}
else
diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c
index 0d63c4f..20b93cd 100644
*** a/src/pl/plpython/plpy_spi.c
--- b/src/pl/plpython/plpy_spi.c
*************** PLy_spi_execute_query(char *query, long
*** 338,344 ****
int rv;
volatile MemoryContext oldcontext;
volatile ResourceOwner oldowner;
! PyObject *ret;
oldcontext = CurrentMemoryContext;
oldowner = CurrentResourceOwner;
--- 338,344 ----
int rv;
volatile MemoryContext oldcontext;
volatile ResourceOwner oldowner;
! PyObject *ret = NULL;
oldcontext = CurrentMemoryContext;
oldowner = CurrentResourceOwner;
*************** PLy_spi_execute_query(char *query, long
*** 362,367 ****
--- 362,368 ----
if (rv < 0)
{
+ Py_XDECREF(ret);
PLy_exception_set(PLy_exc_spi_error,
"SPI_execute failed: %s",
SPI_result_code_string(rv));
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index d5cac9f..9838789 100644
*** a/src/pl/plpython/plpy_typeio.c
--- b/src/pl/plpython/plpy_typeio.c
*************** PLyList_FromArray(PLyDatumToOb *arg, Dat
*** 558,563 ****
--- 558,565 ----
length = ARR_DIMS(array)[0];
lbound = ARR_LBOUND(array)[0];
list = PyList_New(length);
+ if (list == NULL)
+ elog(ERROR, "could not transform Python list to array");
for (i = 0; i < length; i++)
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
