[Asterisk-code-review] Add data buffer API to store packets. (asterisk[15])

Kevin Harwell asteriskteam at digium.com
Fri Mar 23 11:52:15 CDT 2018


Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/8604 )

Change subject: Add data buffer API to store packets.
......................................................................


Patch Set 7: -Code-Review

(3 comments)

https://gerrit.asterisk.org/#/c/8604/7/main/data_buffer.c
File main/data_buffer.c:

https://gerrit.asterisk.org/#/c/8604/7/main/data_buffer.c@112
PS7, Line 112: 			buffer_payload = data_buffer_payload_alloc(NULL, -1);
             : 			if (buffer_payload) {
             : 				AST_LIST_INSERT_TAIL(&buffer->cached_payloads, buffer_payload, list);
             : 				buffer->cache_count++;
             : 			}
             : 
             : 			i++;
> I was worried that, if something went wrong, it would get stuck in an infin
hrm that's a good point. I'd say either leave as is then or just break on error.


https://gerrit.asterisk.org/#/c/8604/7/main/data_buffer.c@125
PS7, Line 125: 			buffer_payload = AST_LIST_REMOVE_HEAD(&buffer->cached_payloads, list);
             : 			if (buffer_payload) {
             : 				ast_free(buffer_payload);
             : 				buffer->cache_count--;
             : 			}
             : 
             : 			i--;
> Similar here, to the above. Unlikely, but possible for the number of items 
Based on your comment above the same thing applies here:

"I'd say either leave as is then or just break on error."


https://gerrit.asterisk.org/#/c/8604/7/main/data_buffer.c@178
PS7, Line 178: 			if (remove) {
> This one is for the buffer, not the cache :)
Dang it I keep doing that! Ignore me. Matter of fact it sounds like you can ignore most of these "findings". I'll let you decide.



-- 
To view, visit https://gerrit.asterisk.org/8604
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff13c5d4795d52356959fe2a57360cd57dfade07
Gerrit-Change-Number: 8604
Gerrit-PatchSet: 7
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Comment-Date: Fri, 23 Mar 2018 16:52:15 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180323/12a5de34/attachment.html>


More information about the asterisk-code-review mailing list