Hi,
On 2018-10-15 21:50:51 -0700, Andres Freund wrote:
> .data 0000000000000028 spi_printtupDR
> .data 0000000000000028 printsimpleDR
> .data 0000000000000028 donothingDR
> .data 0000000000000028 debugtupDR
>
> These we could actually make constant, but CreateDestReceiver() as an
> API makes that inconvenient. They also are pretty darn small... There's
> a security benefit in making them constant and casting the constness
> away - I think that might not be insane.
I.e. do something like the attached.
Greetings,
Andres Freund
diff --git a/src/backend/tcop/dest.c b/src/backend/tcop/dest.c
index c95a4d519de..6960518eca1 100644
--- a/src/backend/tcop/dest.c
+++ b/src/backend/tcop/dest.c
@@ -67,28 +67,33 @@ donothingCleanup(DestReceiver *self)
* static DestReceiver structs for dest types needing no local state
* ----------------
*/
-static DestReceiver donothingDR = {
+static const DestReceiver donothingDR = {
donothingReceive, donothingStartup, donothingCleanup, donothingCleanup,
DestNone
};
-static DestReceiver debugtupDR = {
+static const DestReceiver debugtupDR = {
debugtup, debugStartup, donothingCleanup, donothingCleanup,
DestDebug
};
-static DestReceiver printsimpleDR = {
+static const DestReceiver printsimpleDR = {
printsimple, printsimple_startup, donothingCleanup, donothingCleanup,
DestRemoteSimple
};
-static DestReceiver spi_printtupDR = {
+static const DestReceiver spi_printtupDR = {
spi_printtup, spi_dest_startup, donothingCleanup, donothingCleanup,
DestSPI
};
-/* Globally available receiver for DestNone */
-DestReceiver *None_Receiver = &donothingDR;
+/*
+ * Globally available receiver for DestNone.
+ *
+ * It's ok to cast the constness away as any modification of the none receiver
+ * would be a bug (which gets easier to catch this way).
+ */
+DestReceiver *None_Receiver = (DestReceiver *) &donothingDR;
/* ----------------
@@ -108,6 +113,11 @@ BeginCommand(const char *commandTag, CommandDest dest)
DestReceiver *
CreateDestReceiver(CommandDest dest)
{
+ /*
+ * It's ok to cast the constness away as any modification of the none receiver
+ * would be a bug (which gets easier to catch this way).
+ */
+
switch (dest)
{
case DestRemote:
@@ -115,16 +125,16 @@ CreateDestReceiver(CommandDest dest)
return printtup_create_DR(dest);
case DestRemoteSimple:
- return &printsimpleDR;
+ return (DestReceiver *) &printsimpleDR;
case DestNone:
- return &donothingDR;
+ return (DestReceiver *) &donothingDR;
case DestDebug:
- return &debugtupDR;
+ return (DestReceiver *) &debugtupDR;
case DestSPI:
- return &spi_printtupDR;
+ return (DestReceiver *) &spi_printtupDR;
case DestTuplestore:
return CreateTuplestoreDestReceiver();
@@ -146,7 +156,7 @@ CreateDestReceiver(CommandDest dest)
}
/* should never get here */
- return &donothingDR;
+ pg_unreachable();
}
/* ----------------
