Hi Robin,
On Wed Apr 13 16:57:48 2011, ELMEX wrote:
Show quoted text> Hi!
>
>
> On Wed, Apr 13, 2011 at 03:18:24PM -0400, Dexter Tad-y via RT wrote:
> > Wed Apr 13 15:18:23 2011: Request 67440 was acted upon.
> > Transaction: Ticket created by DTADY
> > Queue: Object-Event
> > Subject: unreg_cb(undef) deletes all callbacks
> > Broken in: 1.22
> > Severity: Critical
> > Owner: Nobody
> > Requestors: DTADY@cpan.org
> > Status: new
> > Ticket <URL:
https://rt.cpan.org/Ticket/Display.html?id=67440 >
> >
> >
> > Hi,
> >
> > I'm indirectly using Object::Event via AnyEvent::STOMP v0.5.
> >
> > Part of AE::STOMP code unregisters a callback that may or may not be
> > undef at any time.
> >
> > When a client code calls $obj->unreg_cb(undef), ALL of $obj's events
> are
> > cleared! Is this the intended behavior?
>
> Hmm, the behavior is not further specified according to the API
> Documentation.
> So at least passing undef is not within intended usage :)
>
I have submitted a patch to AE::STOMP code that addresses the undef
issue; hopefully part of the next release by the author.
Show quoted text> >
> > The code in question is in the ff. lines:
> >
> > In 1.22, lines 350+...
> > for my $reg (values %$evs) {
> > @$reg = grep { (substr $_->[1], 0, $key_len) ne $key } @$reg;
> > }
> >
> > In 1.21, unreg_cb simply does a:
> > for my $reg (values %$evs) {
> > @$reg = grep { $_->[1] ne $cb } @$reg;
> > }
>
> Alternatively i could catch the error inside unreg_cb with something
> like:
>
> not defined $cb and die;
>
> But I have no such checks in other parts of the code, and I more or
> less
> would like to prevent such checks unless it's a very common error.
>
>
> Greetings,
> Robin
>
IMHO, regarding the unreg_cb() method:
1. from the name itself, the method suggests that it expects a
callback/guard argument
2. the synopsis sample code shows that it expects some parameter
I agree and vote for the check: (defined $cb) or die...
As this is a strong contract for the client-code for supply a valid
defined guard. For me, it should not hurt to include this one small
check just inside unreg_cb() for the benefit of preventing
unwanted/unexpected side-effect: deleted all callbacks.
The AE::STOMP code is also already in question, that's why I submitted a
patch.
Cheers!
Dex
