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

[Olle E Johansson]

> On 2010-07-20 00:42:36, Tilghman Lesher wrote:
> > /trunk/main/manager.c, line 6100
> > <https://reviewboard.asterisk.org/r/421/diff/2/?file=11643#file11643line6100>
> >
> >     I'd tend to quote the name within the string, so if it was passed as a blank or with a leading or trailing space, that becomes more obvious.  For that matter, you could probably add an ast_strip after grabbing the argument, to ensure that any leading or trailing spaces are trimmed.

Done.


> On 2010-07-20 00:42:36, Tilghman Lesher wrote:
> > /trunk/main/manager.c, lines 6101-6102
> > <https://reviewboard.asterisk.org/r/421/diff/2/?file=11643#file11643line6101>
> >
> >     It's not necessary to set the buffer if you're returning -1:  it will be set as a condition of that return value.

Fixed.


> On 2010-07-20 00:42:36, Tilghman Lesher wrote:
> > /trunk/main/manager.c, line 6106
> > <https://reviewboard.asterisk.org/r/421/diff/2/?file=11643#file11643line6106>
> >
> >     ast_strip(), again

Fixed.


> On 2010-07-20 00:42:36, Tilghman Lesher wrote:
> > /trunk/main/manager.c, lines 6110-6111
> > <https://reviewboard.asterisk.org/r/421/diff/2/?file=11643#file11643line6110>
> >
> >     Again, setting the buffer is redundant with this return value.

Fixed.


> On 2010-07-20 00:42:36, Tilghman Lesher wrote:
> > /trunk/main/manager.c, line 6122
> > <https://reviewboard.asterisk.org/r/421/diff/2/?file=11643#file11643line6122>
> >
> >     I would suggest that you either 1) implement only the read2 variant, which uses a dynamic string buffer, or 2) set the read_max element, to allow for maximum efficiency of the variable substitution routines.

Done.


- Olle E


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


On 2010-07-17 04:01:35, Olle E Johansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/421/
> -----------------------------------------------------------
> 
> (Updated 2010-07-17 04:01:35)
> 
> 
> 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 277702 
> 
> 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
> 
>