Subject: Re: appendBinaryStringInfo stuff

From: Peter Eisentraut <[email-masked]>
Date: Fri, 10 Feb 2023 13:15:54 +0100

On 19.12.22 07:13, Peter Eisentraut wrote:
> Also, the argument type of appendBinaryStringInfo() is char *.  There is 
> some code that uses this function to assemble some kind of packed binary 
> layout, which requires a bunch of casts because of this.  I think 
> functions taking binary data plus length should take void * instead, 
> like memcpy() for example.

I found a little follow-up for this one: Make the same change to 
pq_sendbytes(), which is a thin wrapper around appendBinaryStringInfo(). 
  This would allow getting rid of further casts at call sites.
From 0aed766e046ba71b01093b44f0cae3a4098f1e5b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email-masked]>
Date: Fri, 10 Feb 2023 13:11:09 +0100
Subject: [PATCH] Change argument type of pq_sendbytes from char * to void *

---
 src/backend/libpq/pqformat.c            |  2 +-
 src/backend/utils/adt/array_userfuncs.c | 11 +++++------
 src/backend/utils/adt/uuid.c            |  2 +-
 src/backend/utils/adt/varbit.c          |  2 +-
 src/include/libpq/pqformat.h            |  2 +-
 5 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c
index b7e2c7b6c9..d712c80b5e 100644
--- a/src/backend/libpq/pqformat.c
+++ b/src/backend/libpq/pqformat.c
@@ -123,7 +123,7 @@ pq_beginmessage_reuse(StringInfo buf, char msgtype)
  * --------------------------------
  */
 void
-pq_sendbytes(StringInfo buf, const char *data, int datalen)
+pq_sendbytes(StringInfo buf, const void *data, int datalen)
 {
 	/* use variant that maintains a trailing null-byte, out of caution */
 	appendBinaryStringInfo(buf, data, datalen);
diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index c8a8d33ca3..80750191d8 100644
--- a/src/backend/utils/adt/array_userfuncs.c
+++ b/src/backend/utils/adt/array_userfuncs.c
@@ -654,7 +654,7 @@ array_agg_serialize(PG_FUNCTION_ARGS)
 	pq_sendbyte(&buf, state->typalign);
 
 	/* dnulls */
-	pq_sendbytes(&buf, (char *) state->dnulls, sizeof(bool) * state->nelems);
+	pq_sendbytes(&buf, state->dnulls, sizeof(bool) * state->nelems);
 
 	/*
 	 * dvalues.  By agreement with array_agg_deserialize, when the element
@@ -664,8 +664,7 @@ array_agg_serialize(PG_FUNCTION_ARGS)
 	 * must be sent first).
 	 */
 	if (state->typbyval)
-		pq_sendbytes(&buf, (char *) state->dvalues,
-					 sizeof(Datum) * state->nelems);
+		pq_sendbytes(&buf, state->dvalues, sizeof(Datum) * state->nelems);
 	else
 	{
 		SerialIOData *iodata;
@@ -1097,7 +1096,7 @@ array_agg_array_serialize(PG_FUNCTION_ARGS)
 	if (state->nullbitmap)
 	{
 		Assert(state->aitems > 0);
-		pq_sendbytes(&buf, (char *) state->nullbitmap, (state->aitems + 7) / 8);
+		pq_sendbytes(&buf, state->nullbitmap, (state->aitems + 7) / 8);
 	}
 
 	/* nitems */
@@ -1107,10 +1106,10 @@ array_agg_array_serialize(PG_FUNCTION_ARGS)
 	pq_sendint32(&buf, state->ndims);
 
 	/* dims: XXX should we just send ndims elements? */
-	pq_sendbytes(&buf, (char *) state->dims, sizeof(state->dims));
+	pq_sendbytes(&buf, state->dims, sizeof(state->dims));
 
 	/* lbs */
-	pq_sendbytes(&buf, (char *) state->lbs, sizeof(state->lbs));
+	pq_sendbytes(&buf, state->lbs, sizeof(state->lbs));
 
 	result = pq_endtypsend(&buf);
 
diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c
index 76ff6c533f..4f7aa768fd 100644
--- a/src/backend/utils/adt/uuid.c
+++ b/src/backend/utils/adt/uuid.c
@@ -154,7 +154,7 @@ uuid_send(PG_FUNCTION_ARGS)
 	StringInfoData buffer;
 
 	pq_begintypsend(&buffer);
-	pq_sendbytes(&buffer, (char *) uuid->data, UUID_LEN);
+	pq_sendbytes(&buffer, uuid->data, UUID_LEN);
 	PG_RETURN_BYTEA_P(pq_endtypsend(&buffer));
 }
 
diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c
index 4fde0c1b14..3dbbd1207f 100644
--- a/src/backend/utils/adt/varbit.c
+++ b/src/backend/utils/adt/varbit.c
@@ -685,7 +685,7 @@ varbit_send(PG_FUNCTION_ARGS)
 
 	pq_begintypsend(&buf);
 	pq_sendint32(&buf, VARBITLEN(s));
-	pq_sendbytes(&buf, (char *) VARBITS(s), VARBITBYTES(s));
+	pq_sendbytes(&buf, VARBITS(s), VARBITBYTES(s));
 	PG_RETURN_BYTEA_P(pq_endtypsend(&buf));
 }
 
diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h
index c5644c0061..0d2f958af3 100644
--- a/src/include/libpq/pqformat.h
+++ b/src/include/libpq/pqformat.h
@@ -22,7 +22,7 @@ extern void pq_beginmessage_reuse(StringInfo buf, char msgtype);
 extern void pq_endmessage(StringInfo buf);
 extern void pq_endmessage_reuse(StringInfo buf);
 
-extern void pq_sendbytes(StringInfo buf, const char *data, int datalen);
+extern void pq_sendbytes(StringInfo buf, const void *data, int datalen);
 extern void pq_sendcountedtext(StringInfo buf, const char *str, int slen,
 							   bool countincludesself);
 extern void pq_sendtext(StringInfo buf, const char *str, int slen);
-- 
2.39.1

---

From: Corey Huinker <[email-masked]>
Date: Fri, 10 Feb 2023 14:08:35 -0500

>   This would allow getting rid of further casts at call sites.

+1

Has all the benefits that 54a177a948b0a773c25c6737d1cc3cc49222a526 had.

Passes make check-world.