Skip to content

Commit 02a4896

Browse files
committed
add Perl_magic_freemglob() magic vtable method
S_mg_free_struct() has a workaround to never free mg->mg_ptr for PERL_MAGIC_regex_global. Move this logic into a new magic vtable free method instead, so that S_mg_free_struct() (which gets called for every type of magic) doesn't have the overhead of checking every time for mg->mg_type == PERL_MAGIC_regex_global. [ No, I don't know why PERL_MAGIC_regex_global's vtable and methods are suffixed mglob rather than regex_global or vice versa ]
1 parent 032a491 commit 02a4896

File tree

6 files changed

+29
-8
lines changed

6 files changed

+29
-8
lines changed

embed.fnc

+1
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,7 @@ dp |int |magic_sethint |NN SV* sv|NN MAGIC* mg
13191319
p |int |magic_setisa |NN SV* sv|NN MAGIC* mg
13201320
p |int |magic_setlvref |NN SV* sv|NN MAGIC* mg
13211321
p |int |magic_setmglob |NN SV* sv|NN MAGIC* mg
1322+
p |int |magic_freemglob|NN SV* sv|NN MAGIC* mg
13221323
p |int |magic_setnkeys |NN SV* sv|NN MAGIC* mg
13231324
p |int |magic_setpack |NN SV* sv|NN MAGIC* mg
13241325
p |int |magic_setpos |NN SV* sv|NN MAGIC* mg

embed.h

+1
Original file line numberDiff line numberDiff line change
@@ -1339,6 +1339,7 @@
13391339
#define magic_copycallchecker(a,b,c,d,e) Perl_magic_copycallchecker(aTHX_ a,b,c,d,e)
13401340
#define magic_existspack(a,b) Perl_magic_existspack(aTHX_ a,b)
13411341
#define magic_freearylen_p(a,b) Perl_magic_freearylen_p(aTHX_ a,b)
1342+
#define magic_freemglob(a,b) Perl_magic_freemglob(aTHX_ a,b)
13421343
#define magic_freeovrld(a,b) Perl_magic_freeovrld(aTHX_ a,b)
13431344
#define magic_freeutf8(a,b) Perl_magic_freeutf8(aTHX_ a,b)
13441345
#define magic_get(a,b) Perl_magic_get(aTHX_ a,b)

mg.c

+21-6
Original file line numberDiff line numberDiff line change
@@ -554,12 +554,10 @@ S_mg_free_struct(pTHX_ SV *sv, MAGIC *mg)
554554
if (vtbl && vtbl->svt_free)
555555
vtbl->svt_free(aTHX_ sv, mg);
556556

557-
if (mg->mg_ptr && mg->mg_type != PERL_MAGIC_regex_global) {
558-
if (mg->mg_len > 0)
559-
Safefree(mg->mg_ptr);
560-
else if (mg->mg_len == HEf_SVKEY)
561-
SvREFCNT_dec(MUTABLE_SV(mg->mg_ptr));
562-
}
557+
if (mg->mg_len > 0)
558+
Safefree(mg->mg_ptr);
559+
else if (mg->mg_len == HEf_SVKEY)
560+
SvREFCNT_dec(MUTABLE_SV(mg->mg_ptr));
563561

564562
if (mg->mg_flags & MGf_REFCOUNTED)
565563
SvREFCNT_dec(mg->mg_obj);
@@ -2600,6 +2598,23 @@ Perl_magic_setmglob(pTHX_ SV *sv, MAGIC *mg)
26002598
return 0;
26012599
}
26022600

2601+
2602+
int
2603+
Perl_magic_freemglob(pTHX_ SV *sv, MAGIC *mg)
2604+
{
2605+
PERL_ARGS_ASSERT_MAGIC_FREEMGLOB;
2606+
PERL_UNUSED_ARG(sv);
2607+
2608+
/* glob magic uses mg_len as a string length rather than a buffer
2609+
* length, so we need to free even with mg_len == 0: hence we can't
2610+
* rely on standard magic free handling */
2611+
assert(mg->mg_type == PERL_MAGIC_regex_global && mg->mg_len >= -1);
2612+
Safefree(mg->mg_ptr);
2613+
mg->mg_ptr = NULL;
2614+
return 0;
2615+
}
2616+
2617+
26032618
int
26042619
Perl_magic_setuvar(pTHX_ SV *sv, MAGIC *mg)
26052620
{

mg_vtable.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ EXT_MGVTBL PL_magic_vtables[magic_vtable_max] = {
172172
{ 0, Perl_magic_setisa, 0, Perl_magic_clearisa, 0, 0, 0, 0 },
173173
{ 0, Perl_magic_setisa, 0, 0, 0, 0, 0, 0 },
174174
{ 0, Perl_magic_setlvref, 0, 0, 0, 0, 0, 0 },
175-
{ 0, Perl_magic_setmglob, 0, 0, 0, 0, 0, 0 },
175+
{ 0, Perl_magic_setmglob, 0, 0, Perl_magic_freemglob, 0, 0, 0 },
176176
{ Perl_magic_getnkeys, Perl_magic_setnkeys, 0, 0, 0, 0, 0, 0 },
177177
{ 0, Perl_magic_setnonelem, 0, 0, 0, 0, 0, 0 },
178178
{ 0, 0, 0, 0, Perl_magic_freeovrld, 0, 0, 0 },

proto.h

+3
Original file line numberDiff line numberDiff line change
@@ -1857,6 +1857,9 @@ PERL_CALLCONV int Perl_magic_existspack(pTHX_ SV* sv, const MAGIC* mg);
18571857
PERL_CALLCONV int Perl_magic_freearylen_p(pTHX_ SV* sv, MAGIC* mg);
18581858
#define PERL_ARGS_ASSERT_MAGIC_FREEARYLEN_P \
18591859
assert(sv); assert(mg)
1860+
PERL_CALLCONV int Perl_magic_freemglob(pTHX_ SV* sv, MAGIC* mg);
1861+
#define PERL_ARGS_ASSERT_MAGIC_FREEMGLOB \
1862+
assert(sv); assert(mg)
18601863
PERL_CALLCONV int Perl_magic_freeovrld(pTHX_ SV* sv, MAGIC* mg);
18611864
#define PERL_ARGS_ASSERT_MAGIC_FREEOVRLD \
18621865
assert(sv); assert(mg)

regen/mg_vtable.pl

+2-1
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,8 @@ BEGIN
260260
'isaelem' => {set => 'setisa'},
261261
'arylen' => {get => 'getarylen', set => 'setarylen', const => 1},
262262
'arylen_p' => {clear => 'cleararylen_p', free => 'freearylen_p'},
263-
'mglob' => {set => 'setmglob'},
263+
'mglob' => {set => 'setmglob',
264+
free => 'freemglob' },
264265
'nkeys' => {get => 'getnkeys', set => 'setnkeys'},
265266
'taint' => {get => 'gettaint', set => 'settaint'},
266267
'substr' => {get => 'getsubstr', set => 'setsubstr'},

0 commit comments

Comments
 (0)