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
