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 to the 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