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

Reply via email to