> On Jan. 28, 2014, 4 a.m., wdoekes wrote:
> > /trunk/res/res_sorcery_astdb.c, lines 277-281
> > <https://reviewboard.asterisk.org/r/3161/diff/1/?file=53171#file53171line277>
> >
> >     So, I deduce that:
> >     
> >       tree=="" means tree=="%", but
> >       tree="abc" does not mean tree="abc%"
> >     
> >     ?
> >     
> >     (And if we have ^ and no regex at all, we could skip the regcomp and 
> > regexec. Right?)

Those deductions are correct.

We could skip the regcomp() and regexec() if regex is "^" but the extra 
complexity doesn't seem worth it.


> On Jan. 28, 2014, 4 a.m., wdoekes wrote:
> > /trunk/res/res_sorcery_astdb.c, lines 229-231
> > <https://reviewboard.asterisk.org/r/3161/diff/1/?file=53171#file53171line229>
> >
> >     I'd rather see this in a separate function. It takes up half the space 
> > in this one right now.

Extracted.


> On Jan. 28, 2014, 4 a.m., wdoekes wrote:
> > /trunk/res/res_sorcery_astdb.c, lines 247-250
> > <https://reviewboard.asterisk.org/r/3161/diff/1/?file=53171#file53171line247>
> >
> >     And no need to add % below, right?

Right


> On Jan. 28, 2014, 4 a.m., wdoekes wrote:
> > /trunk/res/res_sorcery_astdb.c, lines 269-270
> > <https://reviewboard.asterisk.org/r/3161/diff/1/?file=53171#file53171line269>
> >
> >     Adding twice? (You don't even have enough space.)

There was enough room in the tree[] declaration.  (The '^' is removed and two 
'%' added plus a nul terminator.)  However, I did fail to recognize that the 
"%%" in the printf format really means only one '%'.


> On Jan. 28, 2014, 4 a.m., wdoekes wrote:
> > /trunk/res/res_sorcery_astdb.c, lines 255-257
> > <https://reviewboard.asterisk.org/r/3161/diff/1/?file=53171#file53171line255>
> >
> >     Your comments are right. Fix the nextchar check and add a quick check 
> > to see if any "|" is found (ignoring parentheses and backslashes for the 
> > sake of simplicity).
> >     
> >     Sounds like not too much work.

I originally wrote most of what the comment describes but then realized that I 
didn't know enough about regex syntax to be sure of my logic.  Also I don't 
think we really want to make this code that smart about regex syntax.  The 
simple prefix case of "^abc" is the driving force of this patch so CLI tab 
completion and PJSIP REGISTER contact processing won't use so much memory and 
CPU.


- rmudgett


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3161/#review10706
-----------------------------------------------------------


On Jan. 27, 2014, 4:06 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3161/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2014, 4:06 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The sorcery astDB wizzard does not handle regex correctly if the pattern 
> begins with an anchor character.
> 
> This patch attempts to convert the anchored regex pattern to a prefix pattern 
> supported by astDB for performance reasons.  If it is not able to convert the 
> pattern it gets all astDB members of the family and then does a normal regex 
> pattern matching on the resulting records retrieved.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_sorcery_astdb.c 406695 
> 
> Diff: https://reviewboard.asterisk.org/r/3161/diff/
> 
> 
> Testing
> -------
> 
> Simple regex patterns that are just prefixes are converted and the astDB just 
> fetches the matching records.  More complex patterns fallback to getting all 
> astDB family records and searching them by regex.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_____________________________________________________________________
-- 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

Reply via email to