From d5f593387ab2037673927d8a0f33c5bcbb60926b Mon Sep 17 00:00:00 2001 From: Yorhel Date: Mon, 10 Feb 2025 07:34:08 +0100 Subject: [PATCH] Abstract and cleanup ugly byte swapping code These macros don't assume alignment and may be somewhat inefficient with all that copying. I'm hoping GCC is able to optimize that crap somewhat. Also the pg type receive functions can not, in fact, assume that their input buffers are properly aligned. That won't necessarily be the case for array elements. --- Makefile.PL | 1 - c/common.c | 19 ++++++++++ c/pgconn.c | 6 +-- c/pgtypes.c | 106 ++++++++++++++++------------------------------------ 4 files changed, 55 insertions(+), 77 deletions(-) diff --git a/Makefile.PL b/Makefile.PL index cb7d1e8..86cce2c 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -3,7 +3,6 @@ use Config; os_unsupported if $^O eq 'MSWin32'; # I don't know on which OS'es the code will work exactly, but this one I can easily rule out. os_unsupported if $Config{ivsize} < 8; -os_unsupported if $Config{byteorder} != 12345678; # pgtypes.c assumes we're little-endian os_unsupported if $Config{usequadmath}; WriteMakefile( diff --git a/c/common.c b/c/common.c index 4468f39..5325354 100644 --- a/c/common.c +++ b/c/common.c @@ -151,3 +151,22 @@ static SV *fustr_done_(pTHX_ fustr *s) { #define fustr_write_ch(a,b) fustr_write_ch_(aTHX_ a,b) #define fustr_write_buf(a,b) fustr_write_buf_(aTHX_ a,b) #define fustr_done(a) fustr_done_(aTHX_ a) + + +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ +#define fu_bswap(bits, out, in) ({ U##bits tmpswap; memcpy(&tmpswap, in, bits>>3); tmpswap = __builtin_bswap##bits(tmpswap); memcpy(out, &tmpswap, bits>>3); }) +#else +#define fu_bswap(bits, out, in) memcpy(out, in, bits>>3) +#endif + +#define fu_frombeT(T, bits, buf) ({ T tmpval; fu_bswap(bits, &tmpval, buf); tmpval; }) +#define fu_frombeI(bits, buf) fu_frombeT(I##bits, bits, buf) +#define fu_frombeU(bits, buf) fu_frombeT(U##bits, bits, buf) + +#define fu_tobeT(T, bits, out, in) ({ T tmpval = in; fu_bswap(bits, out, &tmpval); }) +#define fu_tobeI(bits, out, in) fu_tobeT(I##bits, bits, out, in) +#define fu_tobeU(bits, out, in) fu_tobeT(U##bits, bits, out, in) + +#define fustr_writebeT(T, bits, s, in) fu_tobeT(T, bits, fustr_write_buf(s, bits>>3), in) +#define fustr_writebeI(bits, s, in) fustr_writebeT(I##bits, bits, s, in) +#define fustr_writebeU(bits, s, in) fustr_writebeT(U##bits, bits, s, in) diff --git a/c/pgconn.c b/c/pgconn.c index a5e46a6..bf8695f 100644 --- a/c/pgconn.c +++ b/c/pgconn.c @@ -284,10 +284,10 @@ static void fupg_refresh_types(pTHX_ fupg_conn *c) { int i; for (i=0; intypes; i++) { fupg_type *t = c->types + i; - t->oid = __builtin_bswap32(*((Oid *)PQgetvalue(r, i, 0))); + t->oid = fu_frombeU(32, PQgetvalue(r, i, 0)); snprintf(t->name, sizeof(t->name), "%s", PQgetvalue(r, i, 1)); char typ = *PQgetvalue(r, i, 2); - t->elemoid = __builtin_bswap32(*((Oid *)PQgetvalue(r, i, 3))); + t->elemoid = fu_frombeU(32, PQgetvalue(r, i, 3)); if (t->elemoid) { /* array */ @@ -298,7 +298,7 @@ static void fupg_refresh_types(pTHX_ fupg_conn *c) { t->send = fupg_send_text; t->recv = fupg_recv_text; } else { - /* TODO: records, (multi)ranges, custom overrides, by-name lookup for dynamic-oid types */ + /* TODO: records, domain types, (multi)ranges, custom overrides, by-name lookup for dynamic-oid types */ const fupg_type *builtin = fupg_builtin_byoid(t->oid); if (builtin) { t->send = builtin->send; diff --git a/c/pgtypes.c b/c/pgtypes.c index 968ef5d..5b858be 100644 --- a/c/pgtypes.c +++ b/c/pgtypes.c @@ -5,9 +5,7 @@ typedef struct fupg_recv fupg_recv; * format into the given fustr. */ typedef void (*fupg_send_fn)(pTHX_ const fupg_send *, SV *, fustr *); -/* Receive function, takes a binary string and should return a Perl value. - * libpq guarantees that the given buffer is aligned to MAXIMUM_ALIGNOF. - */ +/* Receive function, takes a binary string and should return a Perl value. */ typedef SV *(*fupg_recv_fn)(pTHX_ const fupg_recv *, const char *, int); struct fupg_send { @@ -75,46 +73,42 @@ SENDFN(bool) { RECVFN(int2) { RLEN(2); - return newSViv((I16)__builtin_bswap16(*((U16 *)buf))); + return newSViv(fu_frombeI(16, buf)); } SENDFN(int2) { SIV(-32768, 32767); - U16 v = __builtin_bswap16((U16)iv); - fustr_write(out, (const char *)&v, 2); + fustr_writebeI(16, out, iv); } RECVFN(int4) { RLEN(4); - return newSViv((I32)__builtin_bswap32(*((U32 *)buf))); + return newSViv(fu_frombeI(32, buf)); } SENDFN(int4) { SIV(-2147483648, 2147483647); - U32 v = __builtin_bswap32((U32)iv); - fustr_write(out, (const char *)&v, 4); + fustr_writebeI(32, out, iv); } RECVFN(int8) { RLEN(8); - return newSViv((I64)__builtin_bswap64(*((U64 *)buf))); + return newSViv(fu_frombeI(64, buf)); } SENDFN(int8) { SIV(IV_MIN, IV_MAX); - U64 v = __builtin_bswap64((U64)SvIV(val)); - fustr_write(out, (const char *)&v, 8); + fustr_writebeI(64, out, iv); } RECVFN(uint4) { RLEN(4); - return newSViv(__builtin_bswap32(*((U32 *)buf))); + return newSViv(fu_frombeU(32, buf)); } SENDFN(uint4) { SIV(0, UINT32_MAX); - U32 v = __builtin_bswap32((U32)iv); - fustr_write(out, (const char *)&v, 4); + fustr_writebeU(32, out, iv); } RECVFN(bytea) { @@ -154,36 +148,22 @@ SENDFN(text) { RECVFN(float4) { RLEN(4); - U32 uv = __builtin_bswap32(*((U32 *)buf)); - float r; - memcpy(&r, &uv, 4); - return newSVnv(r); + return newSVnv(fu_frombeT(float, 32, buf)); } SENDFN(float4) { if (!looks_like_number(val)) fu_confess("Type '%s' (oid %u) expects a number", ctx->name, ctx->oid); - float r = SvNV(val); - U32 uv; - memcpy(&uv, &r, 4); - uv = __builtin_bswap32(uv); - fustr_write(out, (const char *)&uv, 4); + fustr_writebeT(float, 32, out, SvNV(val)); } RECVFN(float8) { RLEN(8); - U64 uv = __builtin_bswap64(*((U64 *)buf)); - double r; - memcpy(&r, &uv, 8); - return newSVnv(r); + return newSVnv(fu_frombeT(double, 64, buf)); } SENDFN(float8) { if (!looks_like_number(val)) fu_confess("Type '%s' (oid %u) expects a number", ctx->name, ctx->oid); - double r = SvNV(val); - U64 uv; - memcpy(&uv, &r, 8); - uv = __builtin_bswap64(uv); - fustr_write(out, (const char *)&uv, 8); + fustr_writebeT(double, 64, out, SvNV(val)); } RECVFN(json) { @@ -226,42 +206,36 @@ SENDFN(jsonpath) { #define ARRAY_MAXDIM 100 -static SV *fupg_recv_array_elem(pTHX_ const fupg_recv *elem, U32 *header, U32 dim, U32 ndim, const char **buf, int *buflen) { - SV *r; +static SV *fupg_recv_array_elem(pTHX_ const fupg_recv *elem, const char *header, U32 dim, U32 ndim, const char **buf, const char *end) { + SV *r = &PL_sv_undef; if (dim == ndim) { - if (*buflen < 4) fu_confess("Invalid array format"); - U32 len; - memcpy(&len, *buf, 4); /* Buffer is not necessarily aligned at this point */ - len = __builtin_bswap32(len); + if (end - *buf < 4) fu_confess("Invalid array format"); + I32 len = fu_frombeI(32, *buf); *buf += 4; - *buflen -= 4; - r = &PL_sv_undef; - if (len != (U32)-1) { - if ((U32)*buflen < len) fu_confess("Invalid array format"); + if (end - *buf < len) fu_confess("Invalid array format"); + if (len >= 0) { r = elem->fn(aTHX_ elem, *buf, len); *buf += len; - *buflen -= len; } } else { - U32 n = __builtin_bswap32(header[dim*2]); + U32 n = fu_frombeU(32, header + dim*8); AV *av = newAV_alloc_x(n); - r = sv_2mortal(newRV_noinc((SV *)av)); + r = sv_2mortal(newRV_noinc((SV *)av)); /* need to mortalize, we may croak */ U32 i; for (i=0; iname, ctx->oid); - U32 *header = (U32 *)buf; - U32 ndim = __builtin_bswap32(header[0]); - // header[1] is hasnull, can safely ignore - Oid elemtype = __builtin_bswap32(header[2]); + U32 ndim = fu_frombeU(32, buf); + // buf+4 is hasnull, can safely ignore + Oid elemtype = fu_frombeU(32, buf+8); if (elemtype != ctx->arrayelem->oid) fu_confess("Array type '%s' (oid %u) expected elements of type %u but got %u", ctx->name, ctx->oid, ctx->arrayelem->oid, elemtype); @@ -269,19 +243,9 @@ RECVFN(array) { if (ndim > ARRAY_MAXDIM) fu_confess("Array value of type '%s' (oid %u) has too many dimensions (%d)", ctx->name, ctx->oid, ndim); if ((U32)len < 12 + ndim*8) fu_confess("Invalid array format for type '%s' (oid %u)", ctx->name, ctx->oid); - buf += 12 + ndim * 8; - len -= 12 + ndim * 8; - header = header + 3; - - /* Arrays need to be created as temporaries so that they are cleaned up - * when the elem recv function croaks. Their refcnt is increased on - * successful return. */ - ENTER; - SAVETMPS; - SV *r = fupg_recv_array_elem(aTHX_ ctx->arrayelem, header, 0, ndim, &buf, &len); - FREETMPS; - LEAVE; - return r; + const char *header = buf + 12; + const char *data = header + ndim * 8; + return fupg_recv_array_elem(aTHX_ ctx->arrayelem, header, 0, ndim, &data, buf+len); } void fupg_send_array_elem(aTHX_ const fupg_send *elem, const U32 *dims, U32 dim, U32 ndim, SV *v, fustr *out, int *hasnull) { @@ -295,8 +259,7 @@ void fupg_send_array_elem(aTHX_ const fupg_send *elem, const U32 *dims, U32 dim, size_t lenoff = fustr_len(out); fustr_write(out, "\0\0\0\0", 4); elem->fn(elem, v, out); - U32 len = __builtin_bswap32(fustr_len(out) - lenoff - 4); - memcpy(fustr_start(out)+lenoff, &len, 4); + fu_tobeU(32, fustr_start(out) + lenoff, fustr_len(out) - lenoff - 4); return; } @@ -342,16 +305,13 @@ SENDFN(array) { if (dims[0] == 0) ndim = 0; /* Write header */ - U32 tmp = __builtin_bswap32(ndim); - fustr_write(out, (const char *)&tmp, 4); + fustr_writebeU(32, out, ndim); fustr_write(out, "\0\0\0\0", 4); /* Placeholder for isnull */ size_t hasnull_off = fustr_len(out) - 1; - tmp = __builtin_bswap32(ctx->arrayelem->oid); - fustr_write(out, (const char *)&tmp, 4); + fustr_writebeU(32, out, ctx->arrayelem->oid); U32 i; for (i=0; ioid == 22 || ctx->oid == 30) fustr_write(out, "\0\0\0\0", 4);