Re: Parent/child context relation in pg_get_backend_memory_contexts()

David Rowley <dgrowleyml@gmail.com> writes:
> I ended up fixing that another way as the above seems to be casting
> away the const for those variables. Instead, I changed the signature
> of the function to:
> static void get_memory_context_name_and_ident(MemoryContext context,
> const char **const name,  const char **const ident);
> which I think takes into account for the call site variables being
> defined as "const char *".

I did not check the history to see quite what happened here,
but Coverity thinks the end result is rather confused,
and I agree:

*** CID 1615190:  Null pointer dereferences  (REVERSE_INULL)
/srv/coverity/git/pgsql-git/postgresql/src/backend/utils/adt/mcxtfuncs.c: 58 in get_memory_context_name_and_ident()
52         *ident = context->ident;
53
54         /*
55          * To be consistent with logging output, we label dynahash contexts with
56          * just the hash table name as with MemoryContextStatsPrint().
57          */
>>>     CID 1615190:  Null pointer dereferences  (REVERSE_INULL)
>>>     Null-checking "ident" suggests that it may be null, but it has already been dereferenced on all paths leading
tothe check. 
58         if (ident && strcmp(*name, "dynahash") == 0)
59         {
60             *name = *ident;
61             *ident = NULL;
62         }
63     }

It is not clear to me exactly which of these pointers should be
presumed to be possibly-null, but certainly testing ident after
storing through it is pretty pointless.  Maybe what was intended
was

-         if (ident && strcmp(*name, "dynahash") == 0)
+         if (*name && strcmp(*name, "dynahash") == 0)

?

            regards, tom lane