Re: Stop stepping on other perl extension's toes

[Torsten Schoenfeld <kaffeetisch gmx de>]

On 25.11.2010 00:43, Florian Ragwitz wrote:
> This set of patches changes the details of how Glib attaches pointers to
> SVs. The intention is to be more robust and improve interoperability with other
> extensions also using the PERL_MAGIC_ext mechanism.

Looks great in general, thanks.  This is good to have not only because 
it fixes the interoperability problem, but also because it encapsulates 
this tricky magic business in a nice bit of API.

Specific comments:

* It looks like gperl_find_mg is missing a couple of braces; I think as 
it is it will always return the first magic struct found.

* SvTYPE is not null-safe, so gperl_find_mg and gperl_remove_mg should 
guard against this.

* You removed the "!sv || !SvROK (sv)" checks in a couple of places 
before calling SvRV (sv).  Since SvRV isn't null-safe either, I think 
these need to stay.

* The new API needs some short POD paragraphs, maybe similar in 
structure to what gperl_set_isa and gperl_prepend_isa have in GType.xs.

* We need to remember to update the version of Glib that Gtk2 requires 
before release.

* I worried a while about the thread-safety implications of having one 
global static MGVTBL.  But after some pondering I now think that this 
exactly what we want: two scalars in two different threads representing 
the same object need to be associated with the same magic pointer so 
that they represent the same underlying GObject.

With the mentioned things fixed, I think this can go in.  Any dissenting 
views, muppet, Kevin?