scan-build plpython stuff
Started by Peter Eisentrautover 8 years ago1 messages
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
Attachments:
0001-Add-compiler-hints-to-PLy_elog.patchtext/plain; charset=UTF-8; name=0001-Add-compiler-hints-to-PLy_elog.patch; x-mac-creator=0; x-mac-type=0Download
From 6dcfacdfab7cf0ed98065984bc4d8e00e94c12e6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@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
0002-PL-Python-Fix-remaining-scan-build-warnings.patchtext/plain; charset=UTF-8; name=0002-PL-Python-Fix-remaining-scan-build-warnings.patch; x-mac-creator=0; x-mac-type=0Download
From 1b634547e006df6e904be7b8a9106c27a8c17c77 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@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