]> granicus.if.org Git - postgresql/commitdiff
Fix miserable coding in pg_stat_get_activity().
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 10 Sep 2016 17:49:04 +0000 (13:49 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 10 Sep 2016 17:49:04 +0000 (13:49 -0400)
Commit dd1a3bccc replaced a test on whether a subroutine returned a
null pointer with a test on whether &pointer->backendStatus was null.
This accidentally failed to fail, at least on common compilers, because
backendStatus is the first field in the struct; but it was surely trouble
waiting to happen.  Commit f91feba87 then messed things up further,
changing the logic to

local_beentry = pgstat_fetch_stat_local_beentry(curr_backend);
if (!local_beentry)
continue;
beentry = &local_beentry->backendStatus;
if (!beentry)
{

where the second "if" is now dead code, so that the intended behavior of
printing a row with "<backend information not available>" cannot occur.

I suspect this is all moot because pgstat_fetch_stat_local_beentry
will never actually return null in this function's usage, but it's still
very poor coding.  Repair back to 9.4 where the original problem was
introduced.

src/backend/utils/adt/pgstatfuncs.c

index 44ccd37e99895b0295927ede0a85bd8e8d7a5a0f..688eea12a93e431e03ae45a1644c37449baed7d5 100644 (file)
@@ -633,15 +633,13 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
                {
                        /* Get specific pid slot */
                        local_beentry = pgstat_fetch_stat_local_beentry(*(int *) (funcctx->user_fctx));
-                       beentry = &local_beentry->backendStatus;
                }
                else
                {
                        /* Get the next one in the list */
                        local_beentry = pgstat_fetch_stat_local_beentry(funcctx->call_cntr + 1);        /* 1-based index */
-                       beentry = &local_beentry->backendStatus;
                }
-               if (!beentry)
+               if (!local_beentry)
                {
                        int                     i;
 
@@ -655,6 +653,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
                        SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
                }
 
+               beentry = &local_beentry->backendStatus;
+
                /* Values available to all callers */
                values[0] = ObjectIdGetDatum(beentry->st_databaseid);
                values[1] = Int32GetDatum(beentry->st_procpid);