From 867543267f9186a2d7d319817e10fcb8f8925d1f Mon Sep 17 00:00:00 2001 From: Yorhel Date: Wed, 12 Feb 2025 18:54:10 +0100 Subject: [PATCH] Fixes for perl 5.36 with multiplicity + memleak in $st->kv methods I always keep messing up the aTHX_ and pTHX_ stuff because my system perl isn't built with multiplicity, and I still haven't found a satisfactory way of finding SV leaks. Valgrind can't track those :( --- FU.xs | 17 ++++++++++++----- c/jsonparse.c | 4 ++-- c/pgconn.c | 14 +++++++------- c/pgst.c | 30 +++++++++++++++--------------- c/pgtypes.c | 6 +++--- t/json_format.t | 1 + 6 files changed, 40 insertions(+), 32 deletions(-) diff --git a/FU.xs b/FU.xs index 95a42b5..ff02104 100644 --- a/FU.xs +++ b/FU.xs @@ -10,6 +10,13 @@ #include "perl.h" #include "XSUB.h" +#ifndef av_push_simple +#define av_push_simple av_push +#endif +#ifndef BOOL_INTERNALS_sv_isbool_true +#define BOOL_INTERNALS_sv_isbool_true(x) SvPVXtrue(x) +#endif + #include "c/khashl.h" #include "c/common.c" #include "c/jsonfmt.c" @@ -128,12 +135,12 @@ void disconnect(fupg_conn *c) void DESTROY(fupg_conn *c) CODE: - fupg_conn_destroy(c); + fupg_conn_destroy(aTHX_ c); void txn(fupg_conn *c) CODE: FUPG_CONN_COOKIE; - ST(0) = fupg_conn_txn(c); + ST(0) = fupg_conn_txn(aTHX_ c); void exec(fupg_conn *c, SV *sv) CODE: @@ -150,7 +157,7 @@ MODULE = FU PACKAGE = FU::Pg::txn void DESTROY(fupg_txn *t) CODE: - fupg_txn_destroy(t); + fupg_txn_destroy(aTHX_ t); void cache(fupg_txn *x, ...) ALIAS: @@ -167,7 +174,7 @@ void status(fupg_txn *t) void txn(fupg_txn *t) CODE: FUPG_TXN_COOKIE; - ST(0) = fupg_txn_txn(t); + ST(0) = fupg_txn_txn(aTHX_ t); void commit(fupg_txn *t) CODE: @@ -267,4 +274,4 @@ void kvh(fupg_st *st) void DESTROY(fupg_st *st) CODE: - fupg_st_destroy(st); + fupg_st_destroy(aTHX_ st); diff --git a/c/jsonparse.c b/c/jsonparse.c index a2d594a..383881f 100644 --- a/c/jsonparse.c +++ b/c/jsonparse.c @@ -240,12 +240,12 @@ static SV *fujson_parse(pTHX_ fujson_parse_ctx *ctx) { if (ctx->end - ctx->buf < 4) return NULL; if (memcmp(ctx->buf, "true", 4) != 0) return NULL; ctx->buf += 4; - return newSV_true(); + return &PL_sv_yes; case 'f': if (ctx->end - ctx->buf < 5) return NULL; if (memcmp(ctx->buf, "false", 5) != 0) return NULL; ctx->buf += 5; - return newSV_false(); + return &PL_sv_no; case 'n': if (ctx->end - ctx->buf < 4) return NULL; if (memcmp(ctx->buf, "null", 4) != 0) return NULL; diff --git a/c/pgconn.c b/c/pgconn.c index 42f9572..c96fdde 100644 --- a/c/pgconn.c +++ b/c/pgconn.c @@ -136,7 +136,7 @@ static SV *fupg_exec_result(pTHX_ PGresult *r) { return ret; } -static void fupg_exec_ok(pTHX_ fupg_conn *c, const char *sql) { +static void fupg_exec_ok(fupg_conn *c, const char *sql) { PGresult *r = PQexec(c->conn, sql); if (!r) fupg_conn_croak(c, "exec"); if (PQresultStatus(r) != PGRES_COMMAND_OK) fupg_result_croak(r, "exec", sql); @@ -190,7 +190,7 @@ static void fupg_conn_disconnect(fupg_conn *c) { * to clean up the prepared statement cache at this point. */ } -static void fupg_conn_destroy(fupg_conn *c) { +static void fupg_conn_destroy(pTHX_ fupg_conn *c) { PQfinish(c->conn); if (c->buf.sv) SvREFCNT_dec(c->buf.sv); safefree(c->types); @@ -242,7 +242,7 @@ static const char *fupg_txn_status(fupg_txn *t) { } } -static void fupg_txn_commit(pTHX_ fupg_txn *t) { +static void fupg_txn_commit(fupg_txn *t) { char cmd[64]; if (t->parent) snprintf(cmd, sizeof(cmd), "RELEASE SAVEPOINT fupg_%"UVuf, t->cookie); else strcpy(cmd, "COMMIT"); @@ -250,7 +250,7 @@ static void fupg_txn_commit(pTHX_ fupg_txn *t) { fupg_exec_ok(t->conn, cmd); } -static void fupg_txn_rollback(pTHX_ fupg_txn *t) { +static void fupg_txn_rollback(fupg_txn *t) { t->cookie = 0; fupg_exec_ok(t->conn, t->rollback_cmd); } @@ -315,7 +315,7 @@ static void fupg_prepared_prune(fupg_conn *c) { } /* Fetch and ref a prepared statement, returns a new object if nothing was cached */ -static fupg_prep *fupg_prepared_ref(fupg_conn *c, const char *query) { +static fupg_prep *fupg_prepared_ref(pTHX_ fupg_conn *c, const char *query) { fupg_prep prep; prep.hash = kh_hash_str(query); prep.query = (char *)query; @@ -418,12 +418,12 @@ static const fupg_type *fupg_lookup_type(pTHX_ fupg_conn *c, int *refresh_done, if ((t = fupg_builtin_byoid(oid))) return t; if (*refresh_done) return NULL; *refresh_done = 1; - fupg_refresh_types(c); + fupg_refresh_types(aTHX_ c); return fupg_type_byoid(c->types, c->ntypes, oid); } -static const fupg_record *fupg_lookup_record(pTHX_ fupg_conn *c, Oid oid) { +static const fupg_record *fupg_lookup_record(fupg_conn *c, Oid oid) { khint_t k = fupg_records_get(c->records, oid); if (k != kh_end(c->records)) return kh_val(c->records, k); diff --git a/c/pgst.c b/c/pgst.c index 256a650..da4701d 100644 --- a/c/pgst.c +++ b/c/pgst.c @@ -34,7 +34,7 @@ static SV *fupg_exec(pTHX_ fupg_conn *c, const char *sql) { case PGRES_TUPLES_OK: break; default: fupg_result_croak(r, "exec", sql); } - SV *ret = fupg_exec_result(r); + SV *ret = fupg_exec_result(aTHX_ r); PQclear(r); return ret; } @@ -60,7 +60,7 @@ static SV *fupg_q(pTHX_ fupg_conn *c, int stflags, const char *query, I32 ax, I3 return fu_selfobj(st, "FU::Pg::st"); } -static void fupg_st_destroy(fupg_st *st) { +static void fupg_st_destroy(pTHX_ fupg_st *st) { int i; if (st->prep) { @@ -89,7 +89,7 @@ static void fupg_st_prepare(pTHX_ fupg_st *st) { if (st->prepared) fu_confess("invalid attempt to re-prepare invalid statement"); if (st->stflags & FUPG_CACHE) - st->prep = fupg_prepared_ref(st->conn, st->query); + st->prep = fupg_prepared_ref(aTHX_ st->conn, st->query); if (st->prep && st->prep->describe) { snprintf(st->name, sizeof(st->name), "fupg%"UVuf, st->prep->name); st->describe = st->prep->describe; @@ -142,7 +142,7 @@ static void fupg_st_prepare(pTHX_ fupg_st *st) { static SV *fupg_st_param_types(pTHX_ fupg_st *st) { fupg_st_prepare(aTHX_ st); int i, nparams = PQnparams(st->describe); - AV *av = newAV_alloc_x(nparams); + AV *av = nparams == 0 ? newAV() : newAV_alloc_x(nparams); for (i=0; idescribe, i))); return sv_2mortal(newRV_noinc((SV *)av)); @@ -151,7 +151,7 @@ static SV *fupg_st_param_types(pTHX_ fupg_st *st) { static SV *fupg_st_columns(pTHX_ fupg_st *st) { fupg_st_prepare(aTHX_ st); int i, nfields = PQnfields(st->describe); - AV *av = newAV_alloc_x(nfields); + AV *av = nfields == 0 ? newAV() : newAV_alloc_x(nfields); for (i=0; idescribe, i); @@ -281,7 +281,7 @@ static void fupg_st_check_dupcols(pTHX_ fupg_st *st, int start) { static SV *fupg_st_exec(pTHX_ fupg_st *st) { fupg_st_execute(aTHX_ st); - return fupg_exec_result(st->result); + return fupg_exec_result(aTHX_ st->result); } static SV *fupg_st_val(pTHX_ fupg_st *st) { @@ -313,7 +313,7 @@ static SV *fupg_st_rowa(pTHX_ fupg_st *st) { fupg_st_execute(aTHX_ st); if (PQntuples(st->result) == 0) fu_confess("Invalid use of $st->rowl() on query returning zero rows"); if (PQntuples(st->result) > 1) fu_confess("Invalid use of $st->rowl() on query returning more than one row"); - AV *av = newAV_alloc_x(st->nfields); + AV *av = st->nfields == 0 ? newAV() : newAV_alloc_x(st->nfields); SV *sv = sv_2mortal(newRV_noinc((SV *)av)); int i; for (i=0; infields; i++) av_push_simple(av, fupg_st_getval(aTHX_ st, 0, i)); @@ -338,10 +338,10 @@ static SV *fupg_st_rowh(pTHX_ fupg_st *st) { static SV *fupg_st_alla(pTHX_ fupg_st *st) { fupg_st_execute(aTHX_ st); int i, j, nrows = PQntuples(st->result); - AV *av = newAV_alloc_x(nrows); + AV *av = nrows == 0 ? newAV() : newAV_alloc_x(nrows); SV *sv = sv_2mortal(newRV_noinc((SV *)av)); for (i=0; infields); + AV *row = st->nfields == 0 ? newAV() : newAV_alloc_x(st->nfields); av_push_simple(av, newRV_noinc((SV *)row)); for (j=0; jnfields; j++) av_push_simple(row, fupg_st_getval(aTHX_ st, i, j)); @@ -353,7 +353,7 @@ static SV *fupg_st_allh(pTHX_ fupg_st *st) { fupg_st_execute(aTHX_ st); fupg_st_check_dupcols(aTHX_ st, 0); int i, j, nrows = PQntuples(st->result); - AV *av = newAV_alloc_x(nrows); + AV *av = nrows == 0 ? newAV() : newAV_alloc_x(nrows); SV *sv = sv_2mortal(newRV_noinc((SV *)av)); for (i=0; iresult); - AV *av = newAV_alloc_x(nrows * st->nfields); + AV *av = nrows == 0 || st->nfields == 0 ? newAV() : newAV_alloc_x(nrows * st->nfields); SV *sv = sv_2mortal(newRV_noinc((SV *)av)); for (i=0; infields; j++) @@ -386,7 +386,7 @@ static SV *fupg_st_kvv(pTHX_ fupg_st *st) { HV *hv = newHV(); SV *sv = sv_2mortal(newRV_noinc((SV *)hv)); for (i=0; ikvv() query results", SvPV_nolen(key)); hv_store_ent(hv, key, st->nfields == 1 ? &PL_sv_yes : fupg_st_getval(aTHX_ st, i, 1), 0); } @@ -400,9 +400,9 @@ static SV *fupg_st_kva(pTHX_ fupg_st *st) { HV *hv = newHV(); SV *sv = sv_2mortal(newRV_noinc((SV *)hv)); for (i=0; ikva() query results", SvPV_nolen(key)); - AV *row = newAV_alloc_x(st->nfields); + AV *row = st->nfields == 1 ? newAV() : newAV_alloc_x(st->nfields-1); hv_store_ent(hv, key, newRV_noinc((SV *)row), 0); for (j=1; jnfields; j++) av_push_simple(row, fupg_st_getval(aTHX_ st, i, j)); @@ -418,7 +418,7 @@ static SV *fupg_st_kvh(pTHX_ fupg_st *st) { HV *hv = newHV(); SV *sv = sv_2mortal(newRV_noinc((SV *)hv)); for (i=0; ikvh() query results", SvPV_nolen(key)); HV *row = newHV(); hv_store_ent(hv, key, newRV_noinc((SV *)row), 0); diff --git a/c/pgtypes.c b/c/pgtypes.c index 6c8e0ad..677d8b5 100644 --- a/c/pgtypes.c +++ b/c/pgtypes.c @@ -265,7 +265,7 @@ RECVFN(array) { return fupg_recv_array_elem(aTHX_ ctx->arrayelem, header, 0, ndim, &data, buf+len); } -void fupg_send_array_elem(aTHX_ const fupg_tio *elem, const U32 *dims, U32 dim, U32 ndim, SV *v, fustr *out, int *hasnull) { +void fupg_send_array_elem(pTHX_ const fupg_tio *elem, const U32 *dims, U32 dim, U32 ndim, SV *v, fustr *out, int *hasnull) { SvGETMAGIC(v); if (dim == ndim) { if (!SvOK(v)) { @@ -275,7 +275,7 @@ void fupg_send_array_elem(aTHX_ const fupg_tio *elem, const U32 *dims, U32 dim, } size_t lenoff = fustr_len(out); fustr_write(out, "\0\0\0\0", 4); - elem->send(elem, v, out); + elem->send(aTHX_ elem, v, out); fu_tobeU(32, fustr_start(out) + lenoff, fustr_len(out) - lenoff - 4); return; } @@ -395,7 +395,7 @@ SENDFN(record) { } size_t lenoff = fustr_len(out); fustr_write(out, "\0\0\0\0", 4); - ctx->record.tio[i].send(ctx->record.tio+i, sv, out); + ctx->record.tio[i].send(aTHX_ ctx->record.tio+i, sv, out); fu_tobeU(32, fustr_start(out) + lenoff, fustr_len(out) - lenoff - 4); } } diff --git a/t/json_format.t b/t/json_format.t index c6c2575..ca43242 100644 --- a/t/json_format.t +++ b/t/json_format.t @@ -1,4 +1,5 @@ use v5.36; +use experimental 'builtin', 'for_list'; use builtin 'true', 'false'; use Test::More; use Tie::Array;