-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/#review697
-----------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-14 18:11:09)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-14 18:48:25.906612)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-14 18:48:27.171197)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/#review699
-----------------------------------------------------------
Most of what I'm pointing out here are very minor problems. Good job here, Tilghman!
There are quite a few places where review board is showing spacing inconsistencies (big red parts) in your new code. Rather than try to point all of them out individually, I'll just let you know that they're there and should be fixed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-14 18:48:27)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-15 12:13:06.728746)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-15 12:13:08.704486)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-15 12:26:54.632859)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-15 12:26:52.803284)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/#review704
-----------------------------------------------------------
I didn't make it all the way through the patch. Please do a pass on the patch for both documentation and other coding guidelines issues.
This should probably return const. Also, full doxygen documentation would be preferred. (The full documentation comment applies to all new API calls in this patch.)
This is not a valid name for a new API call. In fact, it's not even possible that using this would have worked unless you also updated the appropriate .export file.
This is an important fact that should be in the ast_http_send() API docs
- Russell
On 2009-04-15 12:26:54, Tilghman Lesher wrote:
Quote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-15 12:26:54)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
On 2009-04-16 13:49:55, Russell Bryant wrote:
> /trunk/main/http.c, line 283
> <http://reviewboard.digium.com/r/223/diff/4/?file=4189#file4189line283>
>
> This is an important fact that should be in the ast_http_send() API docs
It actually IS there, but since it is out of the ordinary from the way most functions do it, I felt that it was useful to note.
Quote:
On 2009-04-16 13:49:55, Russell Bryant wrote:
> /trunk/main/astobj2.c, lines 460-461
> <http://reviewboard.digium.com/r/223/diff/4/?file=4188#file4188line460>
>
> I'm okay with this and the other changes to astobj2.c, but they should go in separately since they are not related to this functionality.
Probably. I just noticed that if you don't specify the hashing function, then it just uses a single container anyway.
Quote:
On 2009-04-16 13:49:55, Russell Bryant wrote:
> /trunk/include/asterisk/http.h, lines 141-142
> <http://reviewboard.digium.com/r/223/diff/4/?file=4185#file4185line141>
>
> This should probably return const. Also, full doxygen documentation would be preferred. (The full documentation comment applies to all new API calls in this patch.)
Changed. Also added attribute_pure for the compiler.
Quote:
On 2009-04-16 13:49:55, Russell Bryant wrote:
> /trunk/include/asterisk/http.h, line 182
> <http://reviewboard.digium.com/r/223/diff/4/?file=4185#file4185line182>
>
> This is not a valid name for a new API call. In fact, it's not even possible that using this would have worked unless you also updated the appropriate .export file.
Changed.
Quote:
On 2009-04-16 13:49:55, Russell Bryant wrote:
> /trunk/main/ast_expr2.h, lines 92-108
> <http://reviewboard.digium.com/r/223/diff/4/?file=4187#file4187line92>
>
> Unrelated changes
Oops, missed those. Fixed.
Quote:
On 2009-04-16 13:49:55, Russell Bryant wrote:
> /trunk/main/http.c, line 121
> <http://reviewboard.digium.com/r/223/diff/4/?file=4189#file4189line121>
>
> This should probably just be const char text[]
Changed.
Quote:
On 2009-04-16 13:49:55, Russell Bryant wrote:
> /trunk/main/http.c, lines 131-133
> <http://reviewboard.digium.com/r/223/diff/4/?file=4189#file4189line131>
>
> coding guielines
Fixed.
Quote:
On 2009-04-16 13:49:55, Russell Bryant wrote:
> /trunk/main/http.c, line 154
> <http://reviewboard.digium.com/r/223/diff/4/?file=4189#file4189line154>
>
> coding guielines ...
Fixed. Also changed the name to ast_http_manid_from_vars().
- Tilghman
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/#review704
-----------------------------------------------------------
On 2009-04-15 12:26:54, Tilghman Lesher wrote:
Quote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-15 12:26:54)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-17 18:16:57.546834)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-17 18:16:56.235206)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/#review743
-----------------------------------------------------------
Ship it!
Aside from this MAX_SESSIONS thing, if it has been tested to be working, I say go for it.
This looks like a limitation that was not there previously. I also can't tell why it was added. I say we remove it.
- Russell
On 2009-04-17 18:16:57, Tilghman Lesher wrote:
Quote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/
-----------------------------------------------------------
(Updated 2009-04-17 18:16:57)
Review request for Asterisk Developers.
Summary
-------
Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum