default signal closure via bindingset

[Kevin Ryde <user42 zip com au>]

The program below gets two errors

    GLib-CRITICAL **: g_hash_table_lookup: assertion `hash_table != NULL' failed at /home/gg/bug/glib-sig-ret-undef/foo.pl line 44.
    [gperl_value_from_sv] FIXME: unhandled type - 0 ((null) fundamental for (null))

I think the first is gperl_fundamental_wrapper_class_from_type() called
when "wrapper_class_by_type" is NULL.  Is that supposed to be at least
possible?  Maybe when _gperl_sv_from_value_internal() gets a signal's
return value which is a new fundamental type from some C code, when
neither that new type nor anything else has ever yet told to
gperl_register_fundamental_full()?

I think the second is because gperl_signal_class_closure_marshal() tries
to gperl_value_from_sv() into a GValue which is G_TYPE_NONE.  Dunno if a
return_value location like that is supposed to be allowed, but
gtk_binding_entry_activate() calls g_signal_emitv() that way.

I guess gperl_signal_class_closure_marshal() should do the same as
gperl_closure_marshal() does and don't store the sv to return_value if
the latter is G_TYPE_NONE.  (gperl_closure_marshal() is of course what
you get if you specify a class_closure instead of the default name
"do_mysig".)

I suppose the alternative is to look at the signal info and do nothing
at all if the return_type is NONE, no matter what return_value arg the
caller tried to ask for.  But I'm not up with what the duties of a
marshaller are supposed to be.

Incidentally there's only a problem if a do_mysig() like this returns a
non-undef value.  But there's no requirement for handlers or class
handlers to explicitly return undef if the handler is spec'ed as
return_type G_TYPE_NONE, is there?


package Foo;
use strict;
use warnings;
use Gtk2 '-init';

use Glib::Object::Subclass
  'Gtk2::EventBox',
  signals => { mysig => { param_types => [],
                          return_type => undef,
                          flags       => ['run-last','action'],
                        }
             };

Gtk2::Rc->parse_string (<<'HERE');
binding "Foo_keys" {
  bind "x" { "mysig" () }
}
class "Foo" binding "Foo_keys"
HERE

sub do_mysig {
  my ($self) = @_;
  print "hello from do_mysig\n";
  return 1;
}

package main;
use strict;
use warnings;

my $toplevel = Gtk2::Window->new('toplevel');
my $foo = Foo->new;
$toplevel->add ($foo);

my $event = Gtk2::Gdk::Event->new ('key-press');
my $keyval = Gtk2::Gdk->keyval_from_name('x');
my $display = $foo->get_display;
my $keymap = Gtk2::Gdk::Keymap->get_for_display ($display);
my @keys = $keymap->get_entries_for_keyval ($keyval);
@keys or die;
$event->group($keys[0]->{'group'});
$event->hardware_keycode($keys[0]->{'keycode'});

$foo->signal_emit ('key_press_event', $event);

exit 0;

Index: GType.xs
===================================================================
--- GType.xs	(revision 1055)
+++ GType.xs	(working copy)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2003-2005 by the gtk2-perl team (see the file AUTHORS for
+ * Copyright (C) 2003-2005, 2009 by the gtk2-perl team (see the file AUTHORS for
  * the full list)
  *
  * This library is free software; you can redistribute it and/or modify it
@@ -254,10 +254,12 @@
 GPerlValueWrapperClass *
 gperl_fundamental_wrapper_class_from_type (GType gtype)
 {
-	GPerlValueWrapperClass * res;
+	GPerlValueWrapperClass * res = NULL;
 	G_LOCK (wrapper_class_by_type);
-	res = (GPerlValueWrapperClass *)
-		g_hash_table_lookup (wrapper_class_by_type, (gpointer) gtype);
+	if (wrapper_class_by_type) {
+		res = (GPerlValueWrapperClass *)
+		 g_hash_table_lookup (wrapper_class_by_type, (gpointer) gtype);
+	}
 	G_UNLOCK (wrapper_class_by_type);
 	return res;
 }
@@ -992,7 +994,12 @@
 			gperl_run_exception_handlers ();
 
 		} else if (return_value) {
-			gperl_value_from_sv (return_value, POPs);
+			/* Same as gperl_closure_marshal() ... */
+			/* we need to remove the value to from the stack,
+			 * regardless of whether we do anything with it. */
+			SV * sv = POPs;
+			if (G_VALUE_TYPE (return_value))
+				gperl_value_from_sv (return_value, sv);
 			PUTBACK;
 		}
 		SvSetSV (ERRSV, save_errsv);