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

[Kevin Harwell]

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

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


Patch Set 4:

(2 comments)

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

https://gerrit.asterisk.org/#/c/8604/4/main/data_buffer.c@228
PS4, Line 228: 		ast_data_buffer_cache_adjust(buffer);
             : 		buffer_payload = data_buffer_payload_alloc(payload, pos);
             : 		if (!buffer_payload) {
             : 			return -1;
             : 		}
> The reason I put this here is because if for some reason the buffer is not 
If you call the adjust function every time it will only adjust if the sizes differ, so should be fine?

I also wondering now though if we need the cache system at all. Since items are not being "expired" in the primary list and put back into the cache once the buffer is full and the cache is empty it will only use the buffer's head for new objects. So the head is essentially a singular cache at this point.

I say either remove it due to its current use and complexity/overhead. Or we need to rethink how objects in the buffer are removed/expired and put back into the cache.


https://gerrit.asterisk.org/#/c/8604/4/main/data_buffer.c@260
PS4, Line 260: 	if (pos == -1) {
             : 		buffer_payload = AST_LIST_FIRST(&buffer->payloads);
             : 
             : 		if (buffer_payload) {
             : 			return buffer_payload->payload;
             : 		}
             : 	} else {
             : 		AST_LIST_TRAVERSE(&buffer->payloads, buffer_payload, list) {
             : 			if (buffer_payload->pos == pos) {
             : 				return buffer_payload->payload;
             : 			}
             : 		}
             : 	}
> Here, we want to be able to get a payload from any position, which could be
Is this then just an datastore for objects that can be arbitrarily referenced? If so we might want to consider a different container system (which we already have).

I was thinking this was more of an ordered list of things that we'll either always be pulling from the front of, or finding the first object in the sequence to pull and disposing of all in front of that one? Which seems like would be the case for a retransmission buffer.



-- 
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: 4
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: Wed, 21 Mar 2018 17:11:43 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180321/7c4b06c7/attachment.html>