Re: segv on stack extending

[Torsten Schoenfeld <kaffeetisch gmx de>]

Kevin Ryde wrote:

> > But maybe there's a better way?
>
> What about leaving the names on the stack and replacing progressively
> with the results.  (Note the change from PPCODE: to CODE:, the latter
> not using a local "SP" at all and not popping anything until you
> XSRETURN ... umm, if I'm right about that.)

Good idea.  I committed the attached patch.  Thanks!

-Torsten

Index: GObject.xs
===================================================================
RCS file: /cvsroot/gtk2-perl/gtk2-perl-xs/Glib/GObject.xs,v
retrieving revision 1.76
diff -u -d -p -r1.76 GObject.xs
--- GObject.xs  3 Aug 2008 16:01:12 -0000       1.76
+++ GObject.xs  6 Sep 2008 12:58:53 -0000
@@ -1190,16 +1190,21 @@ g_object_get (object, ...)
     PREINIT:
        GValue value = {0,};
        int i;
-    PPCODE:
+    CODE:
+       /* Use CODE: instead of PPCODE: so we can handle the stack ourselves in
+        * order to avoid that xsubs called by g_object_get_property overwrite
+        * what we put on the stack. */
        PERL_UNUSED_VAR (ix);
-       EXTEND (SP, items-1);
        for (i = 1; i < items; i++) {
                char *name = SvPV_nolen (ST (i));
                init_property_value (object, name, &value);
                g_object_get_property (object, name, &value);
-               PUSHs(sv_2mortal(_gperl_sv_from_value_internal(&value, TRUE)));
+               ST (i - 1) =
+                       sv_2mortal (
+                               _gperl_sv_from_value_internal (&value, TRUE));
                g_value_unset (&value);
        }
+       XSRETURN (items - 1);
 
 
 =for apidoc Glib::Object::set
Index: t/5.t
===================================================================
RCS file: /cvsroot/gtk2-perl/gtk2-perl-xs/Glib/t/5.t,v
retrieving revision 1.7
diff -u -d -p -r1.7 5.t
--- t/5.t       9 Sep 2006 15:04:08 -0000       1.7
+++ t/5.t       6 Sep 2008 12:58:53 -0000
@@ -8,8 +8,9 @@
 
 use strict;
 use warnings;
+use Test::More; # for eq_array
 
-print "1..9\n";
+print "1..10\n";
 
 use Glib;
 
@@ -42,10 +43,17 @@ sub INIT_INSTANCE {
 }
 
 sub FINALIZE_INSTANCE {
-   print "ok 8\n";
+   print "ok 9\n";
+}
+
+sub grow_the_stack {
+  1 .. 500;
 }
 
 sub GET_PROPERTY {
+   # grow the stack to trigger reallocation and movement of it in order to test
+   # that Glib::Object->get handles the stack correctly
+   my @list = grow_the_stack();
    77;
 }
 
@@ -79,9 +87,13 @@ package main;
    # set should have bailed out before setting some_string to bar.
    # cannot use get() here, because GET_PROPERTY always returns 77.
    print $my->{some_string} ne 'foo' ? "not " : "", "ok 7\n";
+
+   # verify that fetching multiple properties doesn't corrupt the stack.
+   print eq_array([$my->get("some_string", "some_string")], [77, 77])
+      ? "" : "not ", "ok 8\n";
 }
 
-print "ok 9\n";
+print "ok 10\n";