[asterisk-dev] [Code Review] Dialplan function for manager account checks - AMI_CLIENT()

[Tilghman Lesher]

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



/trunk/main/manager.c
<https://reviewboard.asterisk.org/r/421/#comment5480>

    btw, there's a good reason to implement this as a callback.  There's a possible race condition here that could make this function return the wrong value:
    
    1. Lock container.
    2. Store container version.
    3. Unlock container.
    LOOP:
    4. Lock container.
    5. Compare container version.  If changed, restart loop.  Else get next item in container.
    6. Unlock container.
    7. Return item.
    8. Increment no_sessions.
    9. Goto LOOP.
    
    If the container version changes, which occurs each time that an item is either added to or removed from the container, then the loop could be restarted, which isn't a big deal, if you are, for example, comparing values (it just takes slightly longer), but it's a very big deal if you're incrementing a monotonically incrementing integer, whose value is the entire point of the function.  Each time the container is unlocked, there's a potential for an item to be added or removed from the container at that time.



/trunk/main/manager.c
<https://reviewboard.asterisk.org/r/421/#comment5481>

    You need to assign the return value to args.name, in case there are leading spaces.



/trunk/main/manager.c
<https://reviewboard.asterisk.org/r/421/#comment5482>

    Ditto.



/trunk/main/manager.c
<https://reviewboard.asterisk.org/r/421/#comment5483>

    read_max should be 12, not 2.  2 only covers the cases when the number of sessions < 10.


- Tilghman


On 2010-07-28 04:20:49, Olle E Johansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/421/
> -----------------------------------------------------------
> 
> (Updated 2010-07-28 04:20:49)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Implementation of a dialplan function for checking manager accounts. I've made it extensible so that we can add other parameters when needed. Right now it only checks the number of logged in sessions for a manager account.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/manager.c 280055 
> 
> Diff: https://reviewboard.asterisk.org/r/421/diff
> 
> 
> Testing
> -------
> 
> Tested on my Linux system and it reports manager logins properly - 0, 1 and 2 concurrent sessions...
> 
> 
> Thanks,
> 
> Olle E
> 
>