Clean up some questionable usages of DatumGet* macros

This tidies up some questionable coding which made use of
DatumGetPointer() for Datums being passed into functions where the
parameter is expected to be a cstring.  We saw no compiler warnings with
the old code as the Pointer type used in DatumGetPointer() happens to
be a char * rather than a void *.  However, that's no excuse and we should
be using the correct macro for the job.

Here we also make use of OutputFunctionCall() rather than using
FunctionCall1() directly to call the type's output function.
OutputFunctionCall() is the standard way to do this.  It casts the
returned value to a cstring for us.

In passing get rid of a duplicate call to strlen().  Most compilers will
likely optimize away the 2nd call, but there may be some that won't.  In
any case, this just aligns the code to some other nearby code that already
does this.

Discussion: https://postgr.es/m/CAApHDvq1D=ehZ8hey8Hz67N+_Zth0GHO5wiVCfv1YcGPMXJq0A@mail.gmail.com

diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 96499046d7796d3b0c14e4551b07cf80ba44caa2..4681954b098cd2ec9b96da387c160a4e77af4ff3 100644 (file)
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -634,7 +634,7 @@ range_serialize(Ranges *range)
        for (i = 0; i < nvalues; i++)
        {
            /* don't forget to include the null terminator ;-) */
-           len += strlen(DatumGetPointer(range->values[i])) + 1;
+           len += strlen(DatumGetCString(range->values[i])) + 1;
        }
    }
    else                        /* fixed-length types (even by-reference) */
@@ -695,9 +695,9 @@ range_serialize(Ranges *range)
        }
        else if (typlen == -2)  /* cstring */
        {
-           int         tmp = strlen(DatumGetPointer(range->values[i])) + 1;
+           int         tmp = strlen(DatumGetCString(range->values[i])) + 1;
 
-           memcpy(ptr, DatumGetPointer(range->values[i]), tmp);
+           memcpy(ptr, DatumGetCString(range->values[i]), tmp);
            ptr += tmp;
        }
 
@@ -780,8 +780,10 @@ range_deserialize(int maxvalues, SerializedRanges *serialized)
        }
        else if (typlen == -2)  /* cstring */
        {
-           datalen += MAXALIGN(strlen(DatumGetPointer(ptr)) + 1);
-           ptr += strlen(DatumGetPointer(ptr)) + 1;
+           Size        slen = strlen(DatumGetCString(ptr)) + 1;
+
+           datalen += MAXALIGN(slen);
+           ptr += slen;
        }
    }
 
@@ -830,7 +832,7 @@ range_deserialize(int maxvalues, SerializedRanges *serialized)
 
            memcpy(dataptr, ptr, slen);
            dataptr += MAXALIGN(slen);
-           ptr += (slen);
+           ptr += slen;
        }
 
        /* make sure we haven't overflown the buffer end */
@@ -3032,19 +3034,17 @@ brin_minmax_multi_summary_out(PG_FUNCTION_ARGS)
    idx = 0;
    for (i = 0; i < ranges_deserialized->nranges; i++)
    {
-       Datum       a,
-                   b;
+       char       *a,
+                  *b;
        text       *c;
        StringInfoData str;
 
        initStringInfo(&str);
 
-       a = FunctionCall1(&fmgrinfo, ranges_deserialized->values[idx++]);
-       b = FunctionCall1(&fmgrinfo, ranges_deserialized->values[idx++]);
+       a = OutputFunctionCall(&fmgrinfo, ranges_deserialized->values[idx++]);
+       b = OutputFunctionCall(&fmgrinfo, ranges_deserialized->values[idx++]);
 
-       appendStringInfo(&str, "%s ... %s",
-                        DatumGetPointer(a),
-                        DatumGetPointer(b));
+       appendStringInfo(&str, "%s ... %s", a, b);
 
        c = cstring_to_text(str.data);
 
@@ -3084,7 +3084,7 @@ brin_minmax_multi_summary_out(PG_FUNCTION_ARGS)
 
        a = FunctionCall1(&fmgrinfo, ranges_deserialized->values[idx++]);
 
-       appendStringInfoString(&str, DatumGetPointer(a));
+       appendStringInfoString(&str, DatumGetCString(a));
 
        b = cstring_to_text(str.data);