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 :(
This commit is contained in:
Yorhel 2025-02-12 18:54:10 +01:00
parent 1f7e2de9a0
commit 867543267f
6 changed files with 40 additions and 32 deletions

17
FU.xs
View file

@ -10,6 +10,13 @@
#include "perl.h" #include "perl.h"
#include "XSUB.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/khashl.h"
#include "c/common.c" #include "c/common.c"
#include "c/jsonfmt.c" #include "c/jsonfmt.c"
@ -128,12 +135,12 @@ void disconnect(fupg_conn *c)
void DESTROY(fupg_conn *c) void DESTROY(fupg_conn *c)
CODE: CODE:
fupg_conn_destroy(c); fupg_conn_destroy(aTHX_ c);
void txn(fupg_conn *c) void txn(fupg_conn *c)
CODE: CODE:
FUPG_CONN_COOKIE; FUPG_CONN_COOKIE;
ST(0) = fupg_conn_txn(c); ST(0) = fupg_conn_txn(aTHX_ c);
void exec(fupg_conn *c, SV *sv) void exec(fupg_conn *c, SV *sv)
CODE: CODE:
@ -150,7 +157,7 @@ MODULE = FU PACKAGE = FU::Pg::txn
void DESTROY(fupg_txn *t) void DESTROY(fupg_txn *t)
CODE: CODE:
fupg_txn_destroy(t); fupg_txn_destroy(aTHX_ t);
void cache(fupg_txn *x, ...) void cache(fupg_txn *x, ...)
ALIAS: ALIAS:
@ -167,7 +174,7 @@ void status(fupg_txn *t)
void txn(fupg_txn *t) void txn(fupg_txn *t)
CODE: CODE:
FUPG_TXN_COOKIE; FUPG_TXN_COOKIE;
ST(0) = fupg_txn_txn(t); ST(0) = fupg_txn_txn(aTHX_ t);
void commit(fupg_txn *t) void commit(fupg_txn *t)
CODE: CODE:
@ -267,4 +274,4 @@ void kvh(fupg_st *st)
void DESTROY(fupg_st *st) void DESTROY(fupg_st *st)
CODE: CODE:
fupg_st_destroy(st); fupg_st_destroy(aTHX_ st);

View file

@ -240,12 +240,12 @@ static SV *fujson_parse(pTHX_ fujson_parse_ctx *ctx) {
if (ctx->end - ctx->buf < 4) return NULL; if (ctx->end - ctx->buf < 4) return NULL;
if (memcmp(ctx->buf, "true", 4) != 0) return NULL; if (memcmp(ctx->buf, "true", 4) != 0) return NULL;
ctx->buf += 4; ctx->buf += 4;
return newSV_true(); return &PL_sv_yes;
case 'f': case 'f':
if (ctx->end - ctx->buf < 5) return NULL; if (ctx->end - ctx->buf < 5) return NULL;
if (memcmp(ctx->buf, "false", 5) != 0) return NULL; if (memcmp(ctx->buf, "false", 5) != 0) return NULL;
ctx->buf += 5; ctx->buf += 5;
return newSV_false(); return &PL_sv_no;
case 'n': case 'n':
if (ctx->end - ctx->buf < 4) return NULL; if (ctx->end - ctx->buf < 4) return NULL;
if (memcmp(ctx->buf, "null", 4) != 0) return NULL; if (memcmp(ctx->buf, "null", 4) != 0) return NULL;

View file

@ -136,7 +136,7 @@ static SV *fupg_exec_result(pTHX_ PGresult *r) {
return ret; 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); PGresult *r = PQexec(c->conn, sql);
if (!r) fupg_conn_croak(c, "exec"); if (!r) fupg_conn_croak(c, "exec");
if (PQresultStatus(r) != PGRES_COMMAND_OK) fupg_result_croak(r, "exec", sql); 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. */ * 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); PQfinish(c->conn);
if (c->buf.sv) SvREFCNT_dec(c->buf.sv); if (c->buf.sv) SvREFCNT_dec(c->buf.sv);
safefree(c->types); 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]; char cmd[64];
if (t->parent) snprintf(cmd, sizeof(cmd), "RELEASE SAVEPOINT fupg_%"UVuf, t->cookie); if (t->parent) snprintf(cmd, sizeof(cmd), "RELEASE SAVEPOINT fupg_%"UVuf, t->cookie);
else strcpy(cmd, "COMMIT"); else strcpy(cmd, "COMMIT");
@ -250,7 +250,7 @@ static void fupg_txn_commit(pTHX_ fupg_txn *t) {
fupg_exec_ok(t->conn, cmd); 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; t->cookie = 0;
fupg_exec_ok(t->conn, t->rollback_cmd); 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 */ /* 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; fupg_prep prep;
prep.hash = kh_hash_str(query); prep.hash = kh_hash_str(query);
prep.query = (char *)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 ((t = fupg_builtin_byoid(oid))) return t;
if (*refresh_done) return NULL; if (*refresh_done) return NULL;
*refresh_done = 1; *refresh_done = 1;
fupg_refresh_types(c); fupg_refresh_types(aTHX_ c);
return fupg_type_byoid(c->types, c->ntypes, oid); 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); khint_t k = fupg_records_get(c->records, oid);
if (k != kh_end(c->records)) return kh_val(c->records, k); if (k != kh_end(c->records)) return kh_val(c->records, k);

View file

@ -34,7 +34,7 @@ static SV *fupg_exec(pTHX_ fupg_conn *c, const char *sql) {
case PGRES_TUPLES_OK: break; case PGRES_TUPLES_OK: break;
default: fupg_result_croak(r, "exec", sql); default: fupg_result_croak(r, "exec", sql);
} }
SV *ret = fupg_exec_result(r); SV *ret = fupg_exec_result(aTHX_ r);
PQclear(r); PQclear(r);
return ret; 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"); 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; int i;
if (st->prep) { 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->prepared) fu_confess("invalid attempt to re-prepare invalid statement");
if (st->stflags & FUPG_CACHE) 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) { if (st->prep && st->prep->describe) {
snprintf(st->name, sizeof(st->name), "fupg%"UVuf, st->prep->name); snprintf(st->name, sizeof(st->name), "fupg%"UVuf, st->prep->name);
st->describe = st->prep->describe; 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) { static SV *fupg_st_param_types(pTHX_ fupg_st *st) {
fupg_st_prepare(aTHX_ st); fupg_st_prepare(aTHX_ st);
int i, nparams = PQnparams(st->describe); int i, nparams = PQnparams(st->describe);
AV *av = newAV_alloc_x(nparams); AV *av = nparams == 0 ? newAV() : newAV_alloc_x(nparams);
for (i=0; i<nparams; i++) for (i=0; i<nparams; i++)
av_push_simple(av, newSViv(PQparamtype(st->describe, i))); av_push_simple(av, newSViv(PQparamtype(st->describe, i)));
return sv_2mortal(newRV_noinc((SV *)av)); 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) { static SV *fupg_st_columns(pTHX_ fupg_st *st) {
fupg_st_prepare(aTHX_ st); fupg_st_prepare(aTHX_ st);
int i, nfields = PQnfields(st->describe); int i, nfields = PQnfields(st->describe);
AV *av = newAV_alloc_x(nfields); AV *av = nfields == 0 ? newAV() : newAV_alloc_x(nfields);
for (i=0; i<nfields; i++) { for (i=0; i<nfields; i++) {
HV *hv = newHV(); HV *hv = newHV();
const char *name = PQfname(st->describe, i); const char *name = PQfname(st->describe, 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) { static SV *fupg_st_exec(pTHX_ fupg_st *st) {
fupg_st_execute(aTHX_ 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) { 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); 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) == 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"); 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)); SV *sv = sv_2mortal(newRV_noinc((SV *)av));
int i; int i;
for (i=0; i<st->nfields; i++) av_push_simple(av, fupg_st_getval(aTHX_ st, 0, i)); for (i=0; i<st->nfields; 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) { static SV *fupg_st_alla(pTHX_ fupg_st *st) {
fupg_st_execute(aTHX_ st); fupg_st_execute(aTHX_ st);
int i, j, nrows = PQntuples(st->result); 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)); SV *sv = sv_2mortal(newRV_noinc((SV *)av));
for (i=0; i<nrows; i++) { for (i=0; i<nrows; i++) {
AV *row = newAV_alloc_x(st->nfields); AV *row = st->nfields == 0 ? newAV() : newAV_alloc_x(st->nfields);
av_push_simple(av, newRV_noinc((SV *)row)); av_push_simple(av, newRV_noinc((SV *)row));
for (j=0; j<st->nfields; j++) for (j=0; j<st->nfields; j++)
av_push_simple(row, fupg_st_getval(aTHX_ st, i, 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_execute(aTHX_ st);
fupg_st_check_dupcols(aTHX_ st, 0); fupg_st_check_dupcols(aTHX_ st, 0);
int i, j, nrows = PQntuples(st->result); 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)); SV *sv = sv_2mortal(newRV_noinc((SV *)av));
for (i=0; i<nrows; i++) { for (i=0; i<nrows; i++) {
HV *row = newHV(); HV *row = newHV();
@ -369,7 +369,7 @@ static SV *fupg_st_allh(pTHX_ fupg_st *st) {
static SV *fupg_st_flat(pTHX_ fupg_st *st) { static SV *fupg_st_flat(pTHX_ fupg_st *st) {
fupg_st_execute(aTHX_ st); fupg_st_execute(aTHX_ st);
int i, j, nrows = PQntuples(st->result); int i, j, nrows = PQntuples(st->result);
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)); SV *sv = sv_2mortal(newRV_noinc((SV *)av));
for (i=0; i<nrows; i++) { for (i=0; i<nrows; i++) {
for (j=0; j<st->nfields; j++) for (j=0; j<st->nfields; j++)
@ -386,7 +386,7 @@ static SV *fupg_st_kvv(pTHX_ fupg_st *st) {
HV *hv = newHV(); HV *hv = newHV();
SV *sv = sv_2mortal(newRV_noinc((SV *)hv)); SV *sv = sv_2mortal(newRV_noinc((SV *)hv));
for (i=0; i<nrows; i++) { for (i=0; i<nrows; i++) {
SV *key = fupg_st_getval(aTHX_ st, i, 0); SV *key = sv_2mortal(fupg_st_getval(aTHX_ st, i, 0));
if (hv_exists_ent(hv, key, 0)) fu_confess("Key '%s' is duplicated in $st->kvv() query results", SvPV_nolen(key)); if (hv_exists_ent(hv, key, 0)) fu_confess("Key '%s' is duplicated in $st->kvv() query results", SvPV_nolen(key));
hv_store_ent(hv, key, st->nfields == 1 ? &PL_sv_yes : fupg_st_getval(aTHX_ st, i, 1), 0); 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(); HV *hv = newHV();
SV *sv = sv_2mortal(newRV_noinc((SV *)hv)); SV *sv = sv_2mortal(newRV_noinc((SV *)hv));
for (i=0; i<nrows; i++) { for (i=0; i<nrows; i++) {
SV *key = fupg_st_getval(aTHX_ st, i, 0); SV *key = sv_2mortal(fupg_st_getval(aTHX_ st, i, 0));
if (hv_exists_ent(hv, key, 0)) fu_confess("Key '%s' is duplicated in $st->kva() query results", SvPV_nolen(key)); if (hv_exists_ent(hv, key, 0)) fu_confess("Key '%s' is duplicated in $st->kva() 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); hv_store_ent(hv, key, newRV_noinc((SV *)row), 0);
for (j=1; j<st->nfields; j++) for (j=1; j<st->nfields; j++)
av_push_simple(row, fupg_st_getval(aTHX_ st, i, 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(); HV *hv = newHV();
SV *sv = sv_2mortal(newRV_noinc((SV *)hv)); SV *sv = sv_2mortal(newRV_noinc((SV *)hv));
for (i=0; i<nrows; i++) { for (i=0; i<nrows; i++) {
SV *key = fupg_st_getval(aTHX_ st, i, 0); SV *key = sv_2mortal(fupg_st_getval(aTHX_ st, i, 0));
if (hv_exists_ent(hv, key, 0)) fu_confess("Key '%s' is duplicated in $st->kvh() query results", SvPV_nolen(key)); if (hv_exists_ent(hv, key, 0)) fu_confess("Key '%s' is duplicated in $st->kvh() query results", SvPV_nolen(key));
HV *row = newHV(); HV *row = newHV();
hv_store_ent(hv, key, newRV_noinc((SV *)row), 0); hv_store_ent(hv, key, newRV_noinc((SV *)row), 0);

View file

@ -265,7 +265,7 @@ RECVFN(array) {
return fupg_recv_array_elem(aTHX_ ctx->arrayelem, header, 0, ndim, &data, buf+len); 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); SvGETMAGIC(v);
if (dim == ndim) { if (dim == ndim) {
if (!SvOK(v)) { 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); size_t lenoff = fustr_len(out);
fustr_write(out, "\0\0\0\0", 4); 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); fu_tobeU(32, fustr_start(out) + lenoff, fustr_len(out) - lenoff - 4);
return; return;
} }
@ -395,7 +395,7 @@ SENDFN(record) {
} }
size_t lenoff = fustr_len(out); size_t lenoff = fustr_len(out);
fustr_write(out, "\0\0\0\0", 4); 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); fu_tobeU(32, fustr_start(out) + lenoff, fustr_len(out) - lenoff - 4);
} }
} }

View file

@ -1,4 +1,5 @@
use v5.36; use v5.36;
use experimental 'builtin', 'for_list';
use builtin 'true', 'false'; use builtin 'true', 'false';
use Test::More; use Test::More;
use Tie::Array; use Tie::Array;