[asterisk-dev] [Code Review] Fix handling of backreferences for ENUM lookups

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/187/#review523
-----------------------------------------------------------



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1229>

    Minor note, it's actually backreferences, not backtraces.
    
    Also, if you want to keep this here so that the array length is not a magic number, that's fine, but I would make it a "static const size_t".
    
    Later in the code, it's a bit safer to use ARRAY_LEN(pmatch) instead of max_bt, as you will always be guaranteed to use the right value.



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1230>

    Might as well remove this in passing ...



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1231>

    I suppose one other case to check for would be for the case that only two instances of the delimeter were found (beginning and end).  If that happens, error out.



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1232>

    A couple of style/formatting comments ...
    
    I would do if ((...)) instead of if (0 != (...))
    
    Also, there should be a space after the cast



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1233>

    remove trailing whitespace, and fix references to "backtrace"



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1234>

    spaces around the -



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1235>

    You'll access invalid stack data here if \9 was used



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1236>

    Add a space after the cast
    
    Also, for sanity, I would validate rm_so, as well.



/trunk/main/enum.c
<http://reviewboard.digium.com/r/187/#comment1237>

    space after cast


- Russell


On 2009-03-05 12:44:44, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/187/
> -----------------------------------------------------------
> 
> (Updated 2009-03-05 12:44:44)
> 
> 
> Review request for Asterisk Developers, Russell Bryant and Mark Michelson.
> 
> 
> Summary
> -------
> 
> enum.c did not handle regex backtraces correctly.  
> 
> for example:
> input: +44800123123
> regex: !^\+44800(.*)$!sip:44800\1 at selfnet.at!
> 
> ^\+44800(.*)$ matches to 123123
> 
> The '\1' in the regex is a backtrace that requires a pattern match to be inserted.  The way the code used to work is that it would find the backtrace and insert the entire input string minus the '+'.  This is ghetto...  The regexec() function takes in a variable called pmatch which is an array of structs containing the start and end indexes for each backtrace substring.  The original code actually passed the pmatch array pointer into regexec but never did anything with it.  Now when a backtrace is found, the backtrace number is looked up in the pmatch array and the correct substring is inserted.
> 
> the result was: sip:44804480123123 at selfnet.at because 4480123123 was being inserted into the backtrace instead of the correct match, 123123
> 
> the new result is sip:4480123123 at selfnet.at
> 
> http://tools.ietf.org/html/rfc3403 discusses some of this.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/enum.c 180005 
> 
> Diff: http://reviewboard.digium.com/r/187/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David
> 
>