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 1bba5fa8c816021a8212284b2ba553efee12cb65..5cced3d1dba115cae533d4ac014aa21473688012 100644 (file)
@@ -688,27 +688,17 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
        MemSet(values, 0, sizeof(values));
        MemSet(nulls, 0, sizeof(nulls));
 
-       if (pid != -1)
-       {
-           /* Skip any which are not the one we're looking for. */
-           PgBackendStatus *be = pgstat_fetch_stat_beentry(curr_backend);
-
-           if (!be || be->st_procpid != pid)
-               continue;
-
-       }
-
        /* Get the next one in the list */
        local_beentry = pgstat_fetch_stat_local_beentry(curr_backend);
        if (!local_beentry)
-           continue;
-
-       beentry = &local_beentry->backendStatus;
-       if (!beentry)
        {
            int         i;
 
-           for (i = 0; i < sizeof(nulls) / sizeof(nulls[0]); i++)
+           /* Ignore missing entries if looking for specific PID */
+           if (pid != -1)
+               continue;
+
+           for (i = 0; i < lengthof(nulls); i++)
                nulls[i] = true;
 
            nulls[5] = false;
@@ -718,6 +708,12 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
            continue;
        }
 
+       beentry = &local_beentry->backendStatus;
+
+       /* If looking for specific PID, ignore all the others */
+       if (pid != -1 && beentry->st_procpid != pid)
+           continue;
+
        /* Values available to all callers */
        values[0] = ObjectIdGetDatum(beentry->st_databaseid);
        values[1] = Int32GetDatum(beentry->st_procpid);