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";