Skip to content

Minor XS simplifications #122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 12, 2025
Merged

Minor XS simplifications #122

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
243b574
Remove "perl_" prefix from "perl_require_pv"
JRaspass Aug 20, 2024
58127fa
Use mPUSH macros to replace sv_2mortal
JRaspass Aug 20, 2024
ef97f35
Use -s macros with literals to avoid magic numbers
JRaspass Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 36 additions & 37 deletions DBI.xs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ typedef struct dbi_ima_st {
(SvOK(state) /* SQLSTATE is implemented by driver */ \
? (strEQ(SvPV_nolen(state),"00000") ? &PL_sv_no : sv_mortalcopy(state))\
: (SvTRUE(DBIc_ERR(imp_xxh)) \
? sv_2mortal(newSVpv("S1000",5)) /* General error */ \
? sv_2mortal(newSVpvs("S1000")) /* General error */ \
: &PL_sv_no) /* Success ("00000") */ \
)

Expand Down Expand Up @@ -425,7 +425,7 @@ _join_hash_sorted(HV *hash, char *kv_sep, STRLEN kv_sep_len, char *pair_sep, STR

keys = _sort_hash_keys(hash, num_sort, &total_len);
if (!keys)
return newSVpv("", 0);
return newSVpvs("");

if (!kv_sep_len)
kv_sep_len = strlen(kv_sep);
Expand All @@ -446,7 +446,7 @@ _join_hash_sorted(HV *hash, char *kv_sep, STRLEN kv_sep_len, char *pair_sep, STR

if (!hash_svp) { /* should never happen */
warn("No hash entry with key '%s'", keys[i]);
sv_catpvn(return_sv, "???", 3);
sv_catpvs(return_sv, "???");
continue;
}

Expand All @@ -457,11 +457,11 @@ _join_hash_sorted(HV *hash, char *kv_sep, STRLEN kv_sep_len, char *pair_sep, STR
if (SvOK(*hash_svp)) {
STRLEN hv_val_len;
char *hv_val = SvPV(*hash_svp, hv_val_len);
sv_catpvn(return_sv, "'", 1);
sv_catpvs(return_sv, "'");
sv_catpvn(return_sv, hv_val, hv_val_len);
sv_catpvn(return_sv, "'", 1);
sv_catpvs(return_sv, "'");
}
else sv_catpvn(return_sv, "undef", 5);
else sv_catpvs(return_sv, "undef");
}

if (i < hv_len-1)
Expand Down Expand Up @@ -612,14 +612,14 @@ neatsvpv(SV *sv, STRLEN maxlen) /* return a tidy ascii value, for debugging only
if (SvMAGICAL(sv)) {
if (DBIS_TRACE_LEVEL >= 5) { /* add magic details to help debugging */
MAGIC* mg;
infosv = sv_2mortal(newSVpv(" (magic-",0));
if (SvSMAGICAL(sv)) sv_catpvn(infosv,"s",1);
if (SvGMAGICAL(sv)) sv_catpvn(infosv,"g",1);
if (SvRMAGICAL(sv)) sv_catpvn(infosv,"r",1);
sv_catpvn(infosv,":",1);
infosv = sv_2mortal(newSVpvs(" (magic-"));
if (SvSMAGICAL(sv)) sv_catpvs(infosv, "s");
if (SvGMAGICAL(sv)) sv_catpvs(infosv, "g");
if (SvRMAGICAL(sv)) sv_catpvs(infosv, "r");
sv_catpvs(infosv, ":");
for (mg = SvMAGIC(sv); mg; mg = mg->mg_moremagic)
sv_catpvn(infosv, &mg->mg_type, 1);
sv_catpvn(infosv, ")", 1);
sv_catpvs(infosv, ")");
}
if (SvGMAGICAL(sv) && !PL_dirty)
mg_get(sv); /* trigger magic to FETCH the value */
Expand Down Expand Up @@ -689,7 +689,7 @@ neatsvpv(SV *sv, STRLEN maxlen) /* return a tidy ascii value, for debugging only
SvGROW(nsv, (1+maxlen+1+1));
sv_setpvn(nsv, quote, 1);
sv_catpvn(nsv, v, maxlen-3); /* account for three dots */
sv_catpvn(nsv, "...", 3);
sv_catpvs(nsv, "...");
} else {
SvGROW(nsv, (1+len+1+1));
sv_setpvn(nsv, quote, 1);
Expand Down Expand Up @@ -720,7 +720,7 @@ copy_statement_to_parent(pTHX_ SV *h, imp_xxh_t *imp_xxh)
if (parent && SvROK(parent)) {
SV *tmp_sv = *hv_fetchs((HV*)SvRV(h), "Statement", 1);
if (SvOK(tmp_sv))
(void)hv_store((HV*)SvRV(parent), "Statement", 9, SvREFCNT_inc(tmp_sv), 0);
(void)hv_stores((HV*)SvRV(parent), "Statement", SvREFCNT_inc(tmp_sv));
}
}

Expand Down Expand Up @@ -771,7 +771,7 @@ set_err_sv(SV *h, imp_xxh_t *imp_xxh, SV *err, SV *errstr, SV *state, SV *method
neatsvpv(method,0)
);
PUSHMARK(SP);
XPUSHs(sv_2mortal(newRV_inc((SV*)DBIc_MY_H(imp_xxh))));
mXPUSHs(newRV_inc((SV*)DBIc_MY_H(imp_xxh)));
XPUSHs(err);
XPUSHs(errstr);
XPUSHs(state);
Expand Down Expand Up @@ -813,7 +813,7 @@ set_err_sv(SV *h, imp_xxh_t *imp_xxh, SV *err, SV *errstr, SV *state, SV *method
if (SvTRUE(h_state) && SvTRUE(state) && strNE(SvPV_nolen(h_state), SvPV_nolen(state)))
sv_catpvf(h_errstr, " [state was %s now %s]", SvPV_nolen(h_state), SvPV_nolen(state));
if (strNE(SvPV_nolen(h_errstr), SvPV_nolen(errstr))) {
sv_catpvn(h_errstr, "\n", 1);
sv_catpvs(h_errstr, "\n");
sv_catsv(h_errstr, errstr);
}
}
Expand Down Expand Up @@ -1484,7 +1484,7 @@ dbih_setup_handle(pTHX_ SV *orv, char *imp_class, SV *parent, SV *imp_datasv)
&& SvROK(*tmp_svp) && SvTYPE(SvRV(*tmp_svp)) == SVt_PVHV
) {
/* XXX mirrors behaviour of dbih_set_attr_k() of Callbacks */
(void)hv_store((HV*)SvRV(h), "Callbacks", 9, newRV_inc(SvRV(*tmp_svp)), 0);
(void)hv_stores((HV*)SvRV(h), "Callbacks", newRV_inc(SvRV(*tmp_svp)));
DBIc_set(imp, DBIcf_Callbacks, 1);
}

Expand Down Expand Up @@ -1522,16 +1522,16 @@ dbih_setup_handle(pTHX_ SV *orv, char *imp_class, SV *parent, SV *imp_datasv)
switch (DBIc_TYPE(imp)) {
case DBIt_DB:
/* cache _inner_ handle, but also see quick_FETCH */
(void)hv_store((HV*)SvRV(h), "Driver", 6, newRV_inc(SvRV(parent)), 0);
(void)hv_stores((HV*)SvRV(h), "Driver", newRV_inc(SvRV(parent)));
(void)hv_fetchs((HV*)SvRV(h), "Statement", 1); /* store writable undef */
break;
case DBIt_ST:
DBIc_NUM_FIELDS((imp_sth_t*)imp) = -1;
/* cache _inner_ handle, but also see quick_FETCH */
(void)hv_store((HV*)SvRV(h), "Database", 8, newRV_inc(SvRV(parent)), 0);
(void)hv_stores((HV*)SvRV(h), "Database", newRV_inc(SvRV(parent)));
/* copy (alias) Statement from the sth up into the dbh */
tmp_svp = hv_fetchs((HV*)SvRV(h), "Statement", 1);
(void)hv_store((HV*)SvRV(parent), "Statement", 9, SvREFCNT_inc(*tmp_svp), 0);
(void)hv_stores((HV*)SvRV(parent), "Statement", SvREFCNT_inc(*tmp_svp));
break;
}
}
Expand Down Expand Up @@ -1586,7 +1586,7 @@ static int
dbih_dumpcom(pTHX_ imp_xxh_t *imp_xxh, const char *msg, int level)
{
dMY_CXT;
SV *flags = sv_2mortal(newSVpv("",0));
SV *flags = sv_2mortal(newSVpvs(""));
SV *inner;
static const char pad[] = " ";
if (!msg)
Expand Down Expand Up @@ -1834,7 +1834,7 @@ dbih_get_fbav(imp_sth_t *imp_sth)
"0", 0, "Number of row fields inconsistent with NUM_OF_FIELDS (driver bug)", "", "_get_fbav");
/*
DBIc_NUM_FIELDS(imp_sth) = i;
hv_delete((HV*)SvRV(sth), "NUM_OF_FIELDS", 13, G_DISCARD);
hv_deletes((HV*)SvRV(sth), "NUM_OF_FIELDS", G_DISCARD);
*/
}
/* don't let SvUTF8 flag persist from one row to the next */
Expand Down Expand Up @@ -2151,14 +2151,14 @@ dbih_set_attr_k(SV *h, SV *keysv, int dbikey, SV *valuesv)
dSP;
I32 returns;
TAINT_NOT; /* the require is presumed innocent till proven guilty */
perl_require_pv("DBI/Profile.pm");
require_pv("DBI/Profile.pm");
if (SvTRUE(ERRSV)) {
warn("Can't load %s: %s", profile_class, SvPV_nolen(ERRSV));
valuesv = &PL_sv_undef;
}
else {
PUSHMARK(SP);
XPUSHs(sv_2mortal(newSVpv(profile_class,0)));
mXPUSHs(newSVpv(profile_class, 0));
XPUSHs(valuesv);
PUTBACK;
returns = call_method("_auto_new", G_SCALAR);
Expand All @@ -2173,7 +2173,7 @@ dbih_set_attr_k(SV *h, SV *keysv, int dbikey, SV *valuesv)
if (on && !sv_isobject(valuesv)) {
/* not blessed already - so default to DBI::Profile */
HV *stash;
perl_require_pv(profile_class);
require_pv(profile_class);
stash = gv_stashpv(profile_class, GV_ADDWARN);
sv_bless(valuesv, stash);
}
Expand Down Expand Up @@ -2752,14 +2752,14 @@ log_where(SV *buf, int append, char *prefix, char *suffix, int show_line, int sh
dTHX;
dTHR;
if (!buf)
buf = sv_2mortal(newSVpv("",0));
buf = sv_2mortal(newSVpvs(""));
else if (!append)
sv_setpv(buf,"");
if (CopLINE(PL_curcop)) {
COP *cop;
dbi_caller_string(buf, PL_curcop, prefix, show_line, show_path);
if (show_caller && (cop = dbi_caller_cop())) {
SV *via = sv_2mortal(newSVpv("",0));
SV *via = sv_2mortal(newSVpvs(""));
dbi_caller_string(via, cop, prefix, show_line, show_path);
sv_catpvf(buf, " via %s", SvPV_nolen(via));
}
Expand Down Expand Up @@ -2938,7 +2938,7 @@ dbi_profile(SV *h, imp_xxh_t *imp_xxh, SV *statement_sv, SV *method, NV t1, NV t
EXTEND(SP, 4);
PUSHMARK(SP);
PUSHs(h); /* push inner handle, then others params */
PUSHs( sv_2mortal(newSVpv(method_pv,0)));
mPUSHs(newSVpv(method_pv, 0));
PUTBACK;
SAVE_DEFSV; /* local($_) = $statement */
DEFSV_set(statement_sv);
Expand Down Expand Up @@ -2983,7 +2983,7 @@ dbi_profile(SV *h, imp_xxh_t *imp_xxh, SV *statement_sv, SV *method, NV t1, NV t
else if (isGV(method)) {
/* just using SvPV_nolen(method) sometimes causes an error: */
/* "Can't coerce GLOB to string" so we use gv_efullname() */
SV *tmpsv = sv_2mortal(newSVpv("",0));
SV *tmpsv = sv_2mortal(newSVpvs(""));
gv_efullname4(tmpsv, (GV*)method, "", TRUE);
p = SvPV_nolen(tmpsv);
if (*p == '*') ++p; /* skip past leading '*' glob sigil */
Expand Down Expand Up @@ -3415,7 +3415,7 @@ XS(XS_DBI_dispatch)
keep_error = TRUE;
if (ima_flags & IMA_CLEAR_STMT) {
/* don't use SvOK_off: dbh's Statement may be ref to sth's */
(void)hv_store((HV*)SvRV(h), "Statement", 9, &PL_sv_undef, 0);
(void)hv_stores((HV*)SvRV(h), "Statement", &PL_sv_undef);
}
if (ima_flags & IMA_CLEAR_CACHED_KIDS)
clear_cached_kids(aTHX_ h, imp_xxh, meth_name, trace_flags);
Expand Down Expand Up @@ -3719,7 +3719,7 @@ XS(XS_DBI_dispatch)
imp_msv = (SV*)gv_fetchmethod_autoload(DBIc_IMP_STASH(imp_xxh), "func", FALSE);
if (imp_msv) {
/* driver does have func method so undo the earlier 'func' stack changes */
PUSHs(sv_2mortal(newSVpv(meth_name,0)));
mPUSHs(newSVpv(meth_name, 0));
PUTBACK;
++items;
meth_name = "func";
Expand Down Expand Up @@ -3925,7 +3925,7 @@ XS(XS_DBI_dispatch)
/* and may mess up the error handling below for the commit/rollback */
PUSHMARK(SP);
XPUSHs(h);
XPUSHs(sv_2mortal(newSVpv("AutoCommit",0)));
mXPUSHs(newSVpvs("AutoCommit"));
XPUSHs(&PL_sv_yes);
PUTBACK;
call_method("STORE", G_VOID);
Expand Down Expand Up @@ -4082,7 +4082,7 @@ XS(XS_DBI_dispatch)
);
PUSHMARK(SP);
XPUSHs(msg);
XPUSHs(sv_2mortal(newRV_inc((SV*)DBIc_MY_H(imp_xxh))));
mXPUSHs(newRV_inc((SV*)DBIc_MY_H(imp_xxh)));
XPUSHs( result );
PUTBACK;
items = call_sv(*hook_svp, G_SCALAR);
Expand Down Expand Up @@ -4605,7 +4605,7 @@ _new_handle(class, parent, attr_ref, imp_datasv, imp_class)
PERL_UNUSED_VAR(cv);
}

(void)hv_store((HV*)SvRV(attr_ref), "ImplementorClass", 16, SvREFCNT_inc(imp_class), 0);
(void)hv_stores((HV*)SvRV(attr_ref), "ImplementorClass", SvREFCNT_inc(imp_class));

/* make attr into inner handle by blessing it into class */
sv_bless(attr_ref, class_stash);
Expand Down Expand Up @@ -4711,7 +4711,7 @@ _install_method(dbi_class, meth_name, file, attribs=Nullsv)
{
dMY_CXT;
/* install another method name/interface for the DBI dispatcher */
SV *trace_msg = (DBIS_TRACE_LEVEL >= 10) ? sv_2mortal(newSVpv("",0)) : Nullsv;
SV *trace_msg = (DBIS_TRACE_LEVEL >= 10) ? sv_2mortal(newSVpvs("")) : Nullsv;
CV *cv;
SV **svp;
dbi_ima_t *ima;
Expand Down Expand Up @@ -5576,8 +5576,7 @@ set_err(h, err, errstr=&PL_sv_no, state=&PL_sv_undef, method=&PL_sv_undef, resul
}
else
(void)SvOK_off(*sem_svp);
EXTEND(SP, 1);
PUSHs( result ? result : &PL_sv_undef );
XPUSHs( result ? result : &PL_sv_undef );
}
/* We don't check RaiseError and call die here because that must be */
/* done by returning through dispatch and letting the DBI handle it */
Expand Down