Bug #79769 for Cache-Cache: checks "defined @$" instead of "defined $@"

Thu Sep 20 15:36:58 2012 rjbs [...] cpan.org - Ticket created

Subject:

checks "defined @$" instead of "defined $@"

Well, is this critical? I'm not sure. I thought it was only cosmetic, because it was raising a "you're using defined ARRAY" warning, which is just a warnings. But when I found the error, it was checking "defined @$" when it really wanted to test $@ to see whether an eval completed successfully. This is wrong not only because of the sigil transposition, but also because $@ is set to an empty (defined) string on no-error, meaning that the test doesn't work even with the transposition fixed. _test_sixteen in Cache::CacheTester should instead end like this: ( $ok ) ? $self->ok( ) : $self->not_ok( "couldn't create autopurge cache" ); I couldn't provide a pull request because I didn't see a repo. :-) -- rjbs

Sat Aug 02 05:52:30 2014 fraserbn [...] gmail.com - Correspondence added

From:

fraserbn [...] gmail.com

On Thu Sep 20 15:36:58 2012, RJBS wrote: Show quoted text

> Well, is this critical? I'm not sure. > > I thought it was only cosmetic, because it was raising a "you're using > defined ARRAY" warning, > which is just a warnings. > > But when I found the error, it was checking "defined @$" when it > really wanted to test $@ to see > whether an eval completed successfully. This is wrong not only > because of the sigil > transposition, but also because $@ is set to an empty (defined) string > on no-error, meaning that > the test doesn't work even with the transposition fixed. > > _test_sixteen in Cache::CacheTester should instead end like this: > > ( $ok ) ? > $self->ok( ) : $self->not_ok( "couldn't create autopurge cache" ); > > I couldn't provide a pull request because I didn't see a repo. :-)

..it's doubly critical now, since defined(@array) is an error in 5.21. The attached patch should fix both this ticket and #87244

Subject:

0001-Check-the-value-of-instead-of-using-defined.patch

From 4f8778be9343d2c3e8e43e4666a1845ea35e8667 Mon Sep 17 00:00:00 2001 From: Brian Fraser <fraserbn@gmail.com> Date: Sat, 2 Aug 2014 11:48:29 +0200 Subject: [PATCH] Check the value of $@ instead of using defined(@$) This was two bugs in one. First one is simple; due to a typo, the module was checking @$ instead of $@; in 5.21, defined(@arr) is an exception, so this started blowing up. The second is more of a thinko -- defined($@) after an eval will almost always be true, regardless of what happened inside the eval! $ perl -E 'eval {1}; say defined $@; eval {die}; say defined $@' 1 1 So the correct thing to do is to check for the truthfulness of $@, not the definedness. --- lib/Cache/CacheTester.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Cache/CacheTester.pm b/lib/Cache/CacheTester.pm index 8f64577..b695863 100644 --- a/lib/Cache/CacheTester.pm +++ b/lib/Cache/CacheTester.pm @@ -559,7 +559,7 @@ sub _test_sixteen $cache = $cache->new( { 'auto_purge_interval' => $expires_in } ); }; - ( not defined @$ ) ? + ( not $@ ) ? $self->ok( ) : $self->not_ok( "couldn't create autopurge cache" ); } -- 1.7.12.4 (Apple Git-37)

Sat Aug 02 05:52:30 2014 The RT System itself - Status changed from 'new' to 'open'

Sat Aug 02 09:42:06 2014 JSWARTZ [...] cpan.org - Correspondence added

Cache::Cache is ancient and no longer maintained. Recommend switching to CHI.

Wed Sep 17 10:01:59 2014 rjbs [...] cpan.org - Correspondence added

Fixed in 1.07 -- rjbs

Wed Sep 17 10:02:00 2014 rjbs [...] cpan.org - Status changed from 'open' to 'resolved'