Re: Stop stepping on other perl extension's toes

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?

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]