Bug #63809 for Devel-PPPort: Please provide mg_findext and sv

Fri Dec 10 14:45:47 2010 rafl [...] debian.org - Ticket created

Subject:	Please provide mg_findext and sv_unmagicext
Date:	Fri, 10 Dec 2010 20:44:46 +0100
To:	bug-Devel-PPPort [...] rt.cpan.org
From:	Florian Ragwitz <rafl [...] debian.org>

Commits 39de7f53b474076d5a8e28b5b41fddefd29e45d7 and b83794c7d64c56b8d918c51e93d1136d33fa202b to bleadperl added the following new API functions: ApdR |MAGIC* |mg_findext |NULLOK const SV* sv|int type|NULLOK const MGVTBL *vtbl Apd |int |sv_unmagicext |NN SV *const sv|const int type|NULLOK MGVTBL *vtbl Those functions provide an interface similar to mg_find and sv_unmagic that takes magic virtual tables into account, similar to how there exists an sv_magicext in addition to sv_magic. They are intended to be used by extensions and are mostly useful when attaching PERL_MAGIC_ext to scalars, for example to attach pointers on the C level, without breaking other extensions that might want to attach their own magic to the same scalar. Implementing them on perls that don't have them isn't very hard and the two aforementioned commits show how to do that - by walking the scalar's SvMAGIC list, comparing to the requested MGVTBL and, in case of sv_unmagicext, re-linking that list. If these functions were provided by ppport.h, many modules could get rid of their runtime dependency on XS::Object::Magic as well as their build-dependency on ExtUtils::Depends. Thanks for considering to provide those functions in ppport.h.

Download (untitled)
application/pgp-signature 197b

Message body not shown because it is not plain text.

Sun Apr 10 07:00:53 2011 MHX [...] cpan.org - Correspondence added

On Fri Dec 10 14:45:47 2010, FLORA wrote: Show quoted text

> > Commits 39de7f53b474076d5a8e28b5b41fddefd29e45d7 and > b83794c7d64c56b8d918c51e93d1136d33fa202b to bleadperl added the > following new API functions: > > ApdR |MAGIC* |mg_findext |NULLOK const SV* sv|int type|NULLOK > const MGVTBL *vtbl > Apd |int |sv_unmagicext |NN SV *const sv|const int > type|NULLOK MGVTBL *vtbl > > Those functions provide an interface similar to mg_find and sv_unmagic > that takes magic virtual tables into account, similar to how there > exists an sv_magicext in addition to sv_magic. > > They are intended to be used by extensions and are mostly useful when > attaching PERL_MAGIC_ext to scalars, for example to attach pointers on > the C level, without breaking other extensions that might want to > attach > their own magic to the same scalar. > > Implementing them on perls that don't have them isn't very hard and > the > two aforementioned commits show how to do that - by walking the > scalar's > SvMAGIC list, comparing to the requested MGVTBL and, in case of > sv_unmagicext, re-linking that list. > > If these functions were provided by ppport.h, many modules could get > rid > of their runtime dependency on XS::Object::Magic as well as their > build-dependency on ExtUtils::Depends. > > > Thanks for considering to provide those functions in ppport.h.

Sorry for the long silence... would you mind providing an initial patch for this? It doesn't have to be perfect, but it should at least contain a (some) valid test case(s). I'm happy to do the polishing if necessary. Thanks, Marcus

Sun Apr 10 07:00:53 2011 The RT System itself - Status changed from 'new' to 'open'

Sat Sep 10 15:36:30 2011 MHX [...] cpan.org - Status changed from 'open' to 'stalled'

Tue Aug 28 19:24:28 2012 BULKDD [...] cpan.org - Correspondence added

On Sun Apr 10 07:00:53 2011, MHX wrote: Show quoted text

> Sorry for the long silence... would you mind providing an initial > patch for this? It doesn't have to be perfect, but it should at > least contain a (some) valid test case(s). I'm happy to do the > polishing if necessary. > > Thanks, > Marcus

Show quoted text

_______________________________________________________ here is a mg_findext that I use myself for old Perls.

_______________________________________________________ #define PERL_API_VERSION_LE(R, V, S) (PERL_API_REVISION < (R) || \ (PERL_API_REVISION == (R) && (PERL_API_VERSION < (V) ||\ (PERL_API_VERSION == (V) && (PERL_API_SUBVERSION <= (S)))))) #if PERL_API_VERSION_LE(5, 13, 8) MAGIC * my_find_mg(SV * sv, int type, const MGVTBL *vtbl){ MAGIC *mg; for (mg = SvMAGIC (sv); mg; mg = mg->mg_moremagic) { if (mg->mg_type == type && mg->mg_virtual == vtbl) assert (mg->mg_ptr); return mg; } return NULL; } #define mg_findext(a,b,c) my_find_mg(a,b,c) #endif

______________________________________________________

Tue Aug 28 19:24:28 2012 The RT System itself - Status changed from 'stalled' to 'open'

Sun Sep 09 15:58:25 2012 LEONT [...] cpan.org - Correspondence added

On Tue Aug 28 19:24:28 2012, BULKDD wrote: Show quoted text

> On Sun Apr 10 07:00:53 2011, MHX wrote:

> > Sorry for the long silence... would you mind providing an initial > > patch for this? It doesn't have to be perfect, but it should at > > least contain a (some) valid test case(s). I'm happy to do the > > polishing if necessary. > > > > Thanks, > > Marcus

> > _______________________________________________________ > > here is a mg_findext that I use myself for old Perls. > > _______________________________________________________ > > #define PERL_API_VERSION_LE(R, V, S) (PERL_API_REVISION < (R) || \ > (PERL_API_REVISION == (R) && (PERL_API_VERSION < (V) ||\ > (PERL_API_VERSION == (V) && (PERL_API_SUBVERSION <= (S)))))) > > #if PERL_API_VERSION_LE(5, 13, 8) > MAGIC * my_find_mg(SV * sv, int type, const MGVTBL *vtbl){ > MAGIC *mg; > for (mg = SvMAGIC (sv); mg; mg = mg->mg_moremagic) { > if (mg->mg_type == type && mg->mg_virtual == vtbl) > assert (mg->mg_ptr); > return mg; > } > return NULL; > } > #define mg_findext(a,b,c) my_find_mg(a,b,c) > #endif > ______________________________________________________

Except that that doesn't do the right thing in presence of threads, and it shouldn't assert on mg_ptr. It should be more like: MAGIC * my_find_mg(pTHX_ SV * sv, int type, const MGVTBL *vtbl){ MAGIC *mg; for (mg = SvMAGIC (sv); mg; mg = mg->mg_moremagic) { if (mg->mg_type == type && mg->mg_virtual == vtbl) return mg; } return NULL; } #define mg_findext(a,b,c) my_find_mg(aTHX_ a,b,c) I'd very much appreciate this getting in. I thought it already was and kind of started depending on it (oops). Leon

Mon Sep 10 07:58:56 2012 LEONT [...] cpan.org - Correspondence added

On Sun Sep 09 15:58:25 2012, LEONT wrote: Show quoted text

> Except that that doesn't do the right thing in presence of threads,

On second sight it seems I was exaggerating, this is a form of handling an SV that should work even without the interpreter being around, though that's not guaranteed in any way. Leon

Tue Mar 19 12:18:33 2013 LEONT [...] cpan.org - Correspondence added

Here is an implementation of sv_unmagicext int Perl_sv_unmagicext(pTHX_ SV *const sv, const int type, const MGVTBL *vtbl) { MAGIC* mg; MAGIC** mgp; if (SvTYPE(sv) < SVt_PVMG || !SvMAGIC(sv)) return 0; mgp = &SvMAGICAL(sv); for (mg = *mgp; mg; mg = *mgp) { const MGVTBL* const virt = mg->mg_virtual; if (mg->mg_type == type && virt == vtbl) { *mgp = mg->mg_moremagic; if (virt && virt->svt_free) virt->svt_free(aTHX_ sv, mg); if (mg->mg_ptr && mg->mg_type != PERL_MAGIC_regex_global) { if (mg->mg_len > 0) Safefree(mg->mg_ptr); else if (mg->mg_len == HEf_SVKEY) SvREFCNT_dec(MUTABLE_SV(mg->mg_ptr)); else if (mg->mg_type == PERL_MAGIC_utf8) Safefree(mg->mg_ptr); } if (mg->mg_flags & MGf_REFCOUNTED) SvREFCNT_dec(mg->mg_obj); Safefree(mg); } else mgp = &mg->mg_moremagic; } if (SvMAGIC(sv)) { if (SvMAGICAL(sv)) /* if we're under save_magic, wait for restore_magic; */ mg_magical(sv); /* else fix the flags now */ } else { SvMAGICAL_off(sv); SvFLAGS(sv) |= (SvFLAGS(sv) & (SVp_IOK|SVp_NOK|SVp_POK)) >> PRIVSHIFT; } return 0; } #define sv_unmagicext(sv, type, vtbl) Perl_sv_unmagicext(aTHX_ sv, type, vtbl) Not tested on older perls though.

Tue Feb 11 09:00:10 2014 WolfSage [...] cpan.org - Correspondence added

On Sun Apr 10 07:00:53 2011, MHX wrote: Show quoted text

> On Fri Dec 10 14:45:47 2010, FLORA wrote:

> > > > Commits 39de7f53b474076d5a8e28b5b41fddefd29e45d7 and > > b83794c7d64c56b8d918c51e93d1136d33fa202b to bleadperl added the > > following new API functions: > > > > ApdR |MAGIC* |mg_findext |NULLOK const SV* sv|int type|NULLOK > > const MGVTBL *vtbl > > Apd |int |sv_unmagicext |NN SV *const sv|const int > > type|NULLOK MGVTBL *vtbl > > > > Those functions provide an interface similar to mg_find and sv_unmagic > > that takes magic virtual tables into account, similar to how there > > exists an sv_magicext in addition to sv_magic. > > > > They are intended to be used by extensions and are mostly useful when > > attaching PERL_MAGIC_ext to scalars, for example to attach pointers on > > the C level, without breaking other extensions that might want to > > attach > > their own magic to the same scalar. > > > > Implementing them on perls that don't have them isn't very hard and > > the > > two aforementioned commits show how to do that - by walking the > > scalar's > > SvMAGIC list, comparing to the requested MGVTBL and, in case of > > sv_unmagicext, re-linking that list. > > > > If these functions were provided by ppport.h, many modules could get > > rid > > of their runtime dependency on XS::Object::Magic as well as their > > build-dependency on ExtUtils::Depends. > > > > > > Thanks for considering to provide those functions in ppport.h.

> > Sorry for the long silence... would you mind providing an initial > patch for this? It doesn't have to be perfect, but it should at > least contain a (some) valid test case(s). I'm happy to do the > polishing if necessary. >

Marcus, I've implemented mg_findext here: https://github.com/mhx/Devel-PPPort/pull/2 If this is acceptable, I'll do the same for sv_unmagicext. Thanks, -- Matthew Horsfall (alh)

Sun Mar 16 11:32:12 2014 WolfSage [...] cpan.org - Correspondence added

sv_unmagicext: https://github.com/mhx/Devel-PPPort/commit/e0e5a747e332f70cf51406a47f85aac539714412 mg_findext: https://github.com/mhx/Devel-PPPort/commit/19ffd02f58faff85dfd4f48804f0eee449f579c9 These should be in the next release which I hope to do soon. If you (rafl (and anyone else who would like to)) could review them, I'd appreciate it! Thanks, -- Matthew Horsfall (alh)

Mon Apr 07 21:38:15 2014 WolfSage [...] cpan.org - Correspondence added

This has been releesed as Devel::PPPort 3.22. Thanks!

Mon Apr 07 21:38:16 2014 WolfSage [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #63809 for Devel-PPPort: Please provide mg_findext and sv_unmagicext

Preferred bug tracker