Patch: Irssi::signal_remove to allow removal of handlers created with coderef callbacks

[irssibot]

• • • Issue #798 opened by @shabble

Currently, Irssi::signaladd(*) accepts either a coderef or a string as a callback argument.

Irssi::signal_remove does not allow the removal of handlers installed using a coderef, since the removal code relies on the

sv_func_cmp(f1, f2) macro, which tests for pointer equivalence, and then string equivalence.

Coderefs to the same function may (are) stored in different SV* variables, so the pointer check is insufficient.

The attached patch checks if the function arguments are references, and if so, checks their reference value for equality. This appears to fix the bug.

Also attached is a simple test script showing the difference between handling string and coderef removals.

[irssibot]

• • • Attachment 247 by @shabble

fix-signal-remove-coderef.patch

From f1f66db22e732ca3e5d920c64c8b904e2fb92762 Mon Sep 17 00:00:00 2001
From: Tom Feist <shabble@metavore.org>
Date: Fri, 8 Apr 2011 20:28:41 +0100
Subject: [PATCH] bugfix: allow Irssi::signal_remove to work properly with coderefs

diff --git a/src/perl/perl-signals.c b/src/perl/perl-signals.c
index a455cfd..1652d09 100644
--- a/src/perl/perl-signals.c
+++ b/src/perl/perl-signals.c
@@ -434,8 +434,9 @@ static void perl_signal_remove_list_one(GSList **siglist, PERL_SIGNAL_REC *rec)
 }

 #define sv_func_cmp(f1, f2) \
-   (f1 == f2 || (SvPOK(f1) && SvPOK(f2) && \
-       strcmp((char *) SvPV_nolen(f1), (char *) SvPV_nolen(f2)) == 0))
+  ((SvROK(f1) && SvROK(f2) && SvRV(f1) == SvRV(f2)) || \
+   (SvPOK(f1) && SvPOK(f2) && \
+    strcmp((char *) SvPV_nolen(f1), (char *) SvPV_nolen(f2)) == 0))

 static void perl_signal_remove_list(GSList **list, SV *func)
 {

[irssibot]

• • • Attachment 248 by @shabble

sig_unbind.pl

use strict;
use warnings;

use Irssi (@Irssi::EXPORT_OK);
use Irssi::Irc;
use Irssi::TextUI;

use Data::Dumper;

our $VERSION = '0.1';
our %IRSSI = (
              authors     => 'shabble',
              contact     => 'shabble+irssi@metavore.org',
              name        => '',
              description => '',
              license     => 'Public Domain',
             );

command_bind("dosig_r",
             sub {
                 my $ref = \&cmd_oink;
                 _print("binding oink to $ref");
                 signal_add("command oink", $ref);
             });

command_bind("undosig_r",
             sub {
                 my $ref = \&cmd_oink;

                 _print("unbinding oink from $ref");

                 signal_remove("command oink", $ref);
                 });

command_bind("dosig_s",
             sub {
                 signal_add("command oink", 'cmd_oink');
             });

command_bind("undosig_s",
             sub {
                 signal_remove("command oink", 'cmd_oink');
                 });

sub cmd_oink {
    Irssi::active_win()->print("Oink:");
}

sub _print  {
    Irssi::active_win()->print($_[0]);
}

command("dosig_r");
command("oink");
command("undosig_r");
command("oink");

[irssibot]

• • • Comment 1306 by @shabble

Addendum:

It is possible to remove coderef callbacks as long as you ensure that the same variable is actually used in both cases.

eg: my $ref = \&foo;

signal_add('blah', $ref); signal_remove('blah', $ref);

In this case, the pointer equality test does work. Still not really practical though.