I wanted to make some progress cleaning up some of the warnings produced by scan-build, so I grabbed plpython and fixed all the warnings there.
Attached are two patches, one that decorates PLy_elog() with compiler hints similar to elog(), and one with some very minor tweaks. This fixes all 13 or so (depending on scan-build version) warnings in plpython. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 6dcfacdfab7cf0ed98065984bc4d8e00e94c12e6 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Tue, 22 Aug 2017 20:05:49 -0400 Subject: [PATCH 1/2] Add compiler hints to PLy_elog() Decorate PLy_elog() in a similar way as elog(), to give compilers and static analyzers hints in which cases it does not return. --- src/pl/plpython/plpy_elog.c | 2 +- src/pl/plpython/plpy_elog.h | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c index bb864899f6..e244104fed 100644 --- a/src/pl/plpython/plpy_elog.c +++ b/src/pl/plpython/plpy_elog.c @@ -44,7 +44,7 @@ static bool set_string_attr(PyObject *obj, char *attrname, char *str); * in the context. */ void -PLy_elog(int elevel, const char *fmt,...) +PLy_elog_impl(int elevel, const char *fmt,...) { char *xmsg; char *tbmsg; diff --git a/src/pl/plpython/plpy_elog.h b/src/pl/plpython/plpy_elog.h index e73177d130..e4b30c3cca 100644 --- a/src/pl/plpython/plpy_elog.h +++ b/src/pl/plpython/plpy_elog.h @@ -10,7 +10,33 @@ extern PyObject *PLy_exc_error; extern PyObject *PLy_exc_fatal; extern PyObject *PLy_exc_spi_error; -extern void PLy_elog(int elevel, const char *fmt,...) pg_attribute_printf(2, 3); +/* + * PLy_elog() + * + * See comments at elog() about the compiler hinting. + */ +#ifdef HAVE__VA_ARGS +#ifdef HAVE__BUILTIN_CONSTANT_P +#define PLy_elog(elevel, ...) \ + do { \ + PLy_elog_impl(elevel, __VA_ARGS__); \ + if (__builtin_constant_p(elevel) && (elevel) >= ERROR) \ + pg_unreachable(); \ + } while(0) +#else /* !HAVE__BUILTIN_CONSTANT_P */ +#define PLy_elog(elevel, ...) \ + do { \ + const int elevel_ = (elevel); \ + PLy_elog_impl(elevel_, __VA_ARGS__); \ + if (elevel_ >= ERROR) \ + pg_unreachable(); \ + } while(0) +#endif /* HAVE__BUILTIN_CONSTANT_P */ +#else /* !HAVE__VA_ARGS */ +#define PLy_elog PLy_elog_impl +#endif /* HAVE__VA_ARGS */ + +extern void PLy_elog_impl(int elevel, const char *fmt,...) pg_attribute_printf(2, 3); extern void PLy_exception_set(PyObject *exc, const char *fmt,...) pg_attribute_printf(2, 3); -- 2.14.1
From 1b634547e006df6e904be7b8a9106c27a8c17c77 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Tue, 22 Aug 2017 20:05:49 -0400 Subject: [PATCH 2/2] PL/Python: Fix remaining scan-build warnings Apparently, scan-build thinks that proc->is_setof can change during PLy_exec_function(). To make it clearer, save the value in a local variable. Also add an assertion to clear another warning. --- src/pl/plpython/plpy_exec.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c index 26f61dd0f3..b10b1681f1 100644 --- a/src/pl/plpython/plpy_exec.c +++ b/src/pl/plpython/plpy_exec.c @@ -57,6 +57,7 @@ static void PLy_abort_open_subtransactions(int save_subxact_level); Datum PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) { + bool is_setof = proc->is_setof; Datum rv; PyObject *volatile plargs = NULL; PyObject *volatile plrv = NULL; @@ -73,7 +74,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) PG_TRY(); { - if (proc->is_setof) + if (is_setof) { /* First Call setup */ if (SRF_IS_FIRSTCALL()) @@ -93,6 +94,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) funcctx = SRF_PERCALL_SETUP(); Assert(funcctx != NULL); srfstate = (PLySRFState *) funcctx->user_fctx; + Assert(srfstate != NULL); } if (srfstate == NULL || srfstate->iter == NULL) @@ -125,7 +127,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) * We stay in the SPI context while doing this, because PyIter_Next() * calls back into Python code which might contain SPI calls. */ - if (proc->is_setof) + if (is_setof) { if (srfstate->iter == NULL) { -- 2.14.1
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers