Skip Menu |

This queue is for tickets about the Glib CPAN distribution.

Report information
The Basics
Id: 88533
Status: resolved
Priority: 0/
Queue: Glib
People
Owner: Nobody in particular
Requestors: 'spro^^*%*^6ut# [...] &$%*c
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Mon Sep 09 04:00:32 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Ticket created
Subject: [PATCH] Possible fix for 5.19.4 compatibility
Yes, we’ve done it again. We’ve broken compatibility with Glib. :-) An implementation detail was leaking through. Passing any scalar to a function should make that scalar itself visible as $_[0], so foo($x) will cause \$_[0] to evaluate to the same thing as \$x. This did not work with undef. foo(undef) would make $_[0] refer to a brand new scalar, rather than the one that undef returns. Glib’s accumulator test in 7.t happens to depend on this bug. Or GType.xs does. In any case &PL_sv_undef is being passed as $_[1], and then 7.t expects to assign to it. Where &PL_sv_undef is coming from ultimately I don’t know, but the attached patch creates a brand new scalar in GType.xs, just before calling the accumulator callback, just the way perl used to do. All tests pass. I leave it to you to determine what is the best fix.
Subject: possible glib fix.txt
diff -rup Glib-1.301-gy1yG6/GType.xs Glib-1.301-8jML2z/GType.xs --- Glib-1.301-gy1yG6/GType.xs 2013-06-23 09:55:30.000000000 -0700 +++ Glib-1.301-8jML2z/GType.xs 2013-09-08 16:25:48.000000000 -0700 @@ -1091,7 +1091,10 @@ gperl_real_signal_accumulator (GSignalIn PUSHMARK (SP); PUSHs (sv_2mortal (newSVGSignalInvocationHint (ihint))); - SAVED_STACK_PUSHs (sv_2mortal (gperl_sv_from_value (return_accu))); + sv = gperl_sv_from_value (return_accu); + if (sv == &PL_sv_undef) + sv = newSV (0); + SAVED_STACK_PUSHs (sv_2mortal (sv)); SAVED_STACK_PUSHs (sv_2mortal (gperl_sv_from_value (handler_return))); if (callback->data)
  Tue Sep 10 16:31:20 2013 kaffeetisch [...] gmx.de - Correspondence added
RT-Send-CC: scott [...] asofyet.org
On Mon Sep 09 04:00:32 2013, SPROUT wrote: Show quoted text
> Yes, we’ve done it again. We’ve broken compatibility with Glib. :-)
No worries. Thanks for the report. Show quoted text
> An implementation detail was leaking through. Passing any scalar to a > function should make that scalar itself visible as $_[0], so foo($x) > will cause \$_[0] to evaluate to the same thing as \$x. > > This did not work with undef. foo(undef) would make $_[0] refer to a > brand new scalar, rather than the one that undef returns. > > Glib’s accumulator test in 7.t happens to depend on this bug. Or > GType.xs does. In any case &PL_sv_undef is being passed as $_[1], and > then 7.t expects to assign to it. Where &PL_sv_undef is coming from > ultimately I don’t know, but the attached patch creates a brand new > scalar in GType.xs, just before calling the accumulator callback, just > the way perl used to do. All tests pass.
Interesting. Upon inspection, I think t/7.t is at fault here. The undef appears in the accumulator callback's $_[1] because it is the first time the signal is called, hence no return values have been accumulated yet. The assigment directly to $_[1] appears to be simply a shortcut to avoid having to introduce a new lexical variable. As you say, this only worked by accident. The attached patch should fix this. muppet (CC), what do you think? Do you remember if the assignment to $_[1] in t/7.t line 97 is more than just a shortcut?
Subject: 0001-Tests-do-not-try-to-modify-a-read-only-value.patch
From d55cf959c30f30182cbed14d9f037735e6c5e6a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torsten=20Sch=C3=B6nfeld?= <kaffeetisch@gmx.de> Date: Tue, 10 Sep 2013 22:21:50 +0200 Subject: [PATCH] Tests: do not try to modify a read-only value t/7.t was failing on perl 5.19.4 because it attempted to assign to undef. --- t/7.t | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/t/7.t b/t/7.t index 6203725..1bee20e 100644 --- a/t/7.t +++ b/t/7.t @@ -84,23 +84,23 @@ use Glib::Object::Subclass flags => 'run-last', return_type => 'Glib::Scalar', accumulator => sub { - # the accumulator gets (ihint, return_accu, handler_return) + my ($ihint, $return_accu, $handler_return) = @_; # let's turn the return_accu into a list of all the handlers' # return values. this is weird, but the sort of thing you # might actually want to do. - print "# in accumulator, got $_[2], previously " - . (defined ($_[1]) ? $_[1] : 'undef') + print "# in accumulator, got $handler_return, previously " + . (defined ($return_accu) ? $return_accu : 'undef') . "\n"; - if ('ARRAY' eq ref $_[1]) { - push @{$_[1]}, $_[2]; + if ('ARRAY' eq ref $return_accu) { + push @{$return_accu}, $handler_return; } else { - $_[1] = [$_[2]]; + $return_accu = [$handler_return]; } # we must return two values --- a boolean that says whether # the signal keeps emitting, and the accumulated return value. # we'll stop emitting if the handler returns the magic # value 42. - ($_[2] != 42, $_[1]) + ($handler_return != 42, $return_accu) }, }, }, -- 1.8.1.2
  Tue Sep 10 16:31:21 2013 The RT System itself - Status changed from 'new' to 'open'
  Wed Dec 04 16:16:07 2013 kaffeetisch [...] gmx.de - Correspondence added
RT-Send-CC: scott [...] asofyet.org
The fix has now been committed to the git repository. Thanks for your support.
  Wed Dec 04 16:16:08 2013 kaffeetisch [...] gmx.de - Status changed from 'open' to 'resolved'