[asterisk-dev] [Code Review] Implement ast_channel_search_locked to expensive per-notify channel walk in chan_sip

[Steve Murphy]

On Tue, 2008-11-04 at 15:54 -0800, Brian Degenhardt wrote:
> Sean Bright wrote:
> > Brian Degenhardt wrote:
> >> Russell, at astridevcon we talked about CEL's method of seeing if a
> >> linkedid exists when a channel is destroyed, and how it needs to search
> >> all channels.  Would this be a reasonable substitute to accomplish that
> >> task?
> >> 
> >> Murf was going to do the whole hashtab thing, but this might be good enough.
> >> 
> >> What do you guys think?  Here's the current function, to refresh your
> >> memory:
> > 
> > You should be able to re-write it like this:
> > 
> > /* called whenever a channel is destroyed or a linkedid is changed to
> > potentially emit a CEL_LINKEDID_END event */
> > void ast_cel_check_retire_linkedid(const struct ast_channel *chan)
> > {
> >     auto int linkedid_match(struct ast_channel *c);
> >     int linkedid_match(struct ast_channel *c) {
> >         return (c != chan && c->linkedid && !strcmp(linkedid, c->linkedid));
> >     }
> >     const char *linkedid = chan->linkedid;
> > 
> >     /* make sure we need to do all this work */
> >     if (!ast_strlen_zero(linkedid) && track_event(CEL_LINKEDID_END)) {
> > 	struct ast_channel *tmp = NULL;
> >         if ((tmp = ast_channel_search_locked(linkedid_match))) {
> >             ast_channel_unlock(tmp);
> >             tmp = NULL;
> >         } else {
> >             ast_cel_report_event(chan, CEL_LINKEDID_END, NULL, NULL, NULL);
> >         }
> >     }
> > }
> > 
> > And you're good to go.
> > 
> 
> That's what I was thinking, but I was wondering if Murf and Russell
> could comment on if that approach is adequate, or if we want to maintain
> a hashtab of channels keyed on linkedid, and insert/remove them from there.
> 
> I'm sorta partial to your way, above.  While it's slower, we don't have
> to worry about maintaining the separate data structure which has the
> potential to get out of sync.  Especially since this code only runs if
> you care about LINKEDID_END events.
> 

Brian--

Without any other objections...

I will use the walk for the time being; with the above improvements
that Sean suggests. Sometimes the cheapest implementation is the 
best-- and if it causes problems we can recode to plan b. And if it
ends up being forever sufficient, we saved ourselves an unnecessary
expense. 


murf

> cheers
> -bmd
> 
> _______________________________________________
> --Bandwidth and Colocation Provided by http://www.api-digital.com--
> 
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev
-- 
Steve Murphy
Software Developer
Digium
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3227 bytes
Desc: not available
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20081106/22e6d9b7/attachment.bin