Re: Stop stepping on other perl extension's toes

Torsten Schoenfeld <kaffeetisch gmx de> writes:

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

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.

Oh, bummer. I guess I added that assertion relatively late, and
obviously carelessly.

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

I disagree. See below.

* 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.

Point taken there. However, I think adding those checks again is the
right thing to do. Again, see below.

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

See the commit message. I don't consider the functions I added to be
useful as a public interface. If a module wants to attach random
pointers to arbitrary SVs and doesn't want to actually implement all of
the hairy magic business, I believe it should just be using

The reason for me making the symbols public is because they're needed in
a couple of places other than GObject.xs where Glib and Gtk weren't
reusing the infrastructure of GObject.xs for various reasons. Short of
making those places do their job in a different or leaving them as buggy
as they were, it seemed to be a reasonable thing to do to share the

This is the reason the functions I added are rather lax in their
argument checking. They're supposed to be called from a site already
checking those things, such as gperl_get_object or

* I worried a while about the thread-safety implications of having one
global static MGVTBL.

Don't be. Those two things are really completely unrelated. The MGVTBL
is used to identify an otherwise opaque MAGIC pointer as being our own

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.

I agree. However, if we ever decided to want something different, having
MGVTBLs for every thread would be the wrong thing to do. Instead, we'd
toggle the MGf_DUP flag of the MAGIC and give our vtbl and svt_dup entry
that would, upon thread creation, be called with the magic pointer and
would be free to also create a new either clone the GObject on the C
level and attaching it to the new MAGIC of the thread under
construction, have the cloned SV reference the same GObject as the SV
it's being cloned from, or do whatever.

In addition, an svt_dup callback can also eliminate the need for
tracking all perl gobjects as well as the associated global
lock. However, this series of patches isn't going there just yet.

Attachment: pgpVMKjkLp28g.pgp
Description: PGP signature

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