Add explicit casts in four places to simplehash.h
Hi hackers,
While working on an extension, I found that simplehash.h is missing
explicit casts in four places. Without these casts, compiling code
including simplehash.h yields warnings if the code is compiled with
-Wc++-compat.
PostgreSQL seems to mostly prefer omitting the explicit casts, however
there are many places where an explicit cast is actually used. Among
many others, see e.g.
bool.c:
state = (BoolAggState *) MemoryContextAlloc(agg_context,
sizeof(BoolAggState));
domains.c:
my_extra = (DomainIOData *) MemoryContextAlloc(mcxt,
sizeof(DomainIOData));
What about, while not being strictly necessary for PostgreSQL itself,
also adding such casts to simplehash.h so that it can be used in code
where -Wc++-compat is enabled?
Attached is a small patch that adds the aforementioned casts.
Thanks for your consideration!
--
David Geier
(ServiceNow)
Attachments:
0001-Add-explicit-casts-to-simplehash.h.patchtext/x-patch; charset=UTF-8; name=0001-Add-explicit-casts-to-simplehash.h.patchDownload+4-5
David Geier <geidav.pg@gmail.com> writes:
What about, while not being strictly necessary for PostgreSQL itself,
also adding such casts to simplehash.h so that it can be used in code
where -Wc++-compat is enabled?
Seems reasonable, so done (I fixed one additional spot you missed).
The bigger picture here is that we do actually endeavor to keep
(most of) our headers C++ clean, but our tool cpluspluscheck misses
these problems because it doesn't try to use these macros.
I wonder whether there is a way to do better.
regards, tom lane
On 11/3/22 15:50, Tom Lane wrote:
Seems reasonable, so done (I fixed one additional spot you missed).
Thanks!
The bigger picture here is that we do actually endeavor to keep
(most of) our headers C++ clean, but our tool cpluspluscheck misses
these problems because it doesn't try to use these macros.
I wonder whether there is a way to do better.
What about having a custom header alongside cpluspluscheck which
references all macros we care about?
We could start with the really big macros like the ones from
simplehash.h and add as we go.
I could give this a try if deemed useful.
--
David Geier
(ServiceNow)
David Geier <geidav.pg@gmail.com> writes:
On 11/3/22 15:50, Tom Lane wrote:
The bigger picture here is that we do actually endeavor to keep
(most of) our headers C++ clean, but our tool cpluspluscheck misses
these problems because it doesn't try to use these macros.
I wonder whether there is a way to do better.
What about having a custom header alongside cpluspluscheck which
references all macros we care about?
Can't see that that would help, because of the hand maintenance
required to make it useful.
regards, tom lane