[Asterisk-code-review] media cache: Add a core API and facade for a backend agnosti... (asterisk[master])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: media cache: Add a core API and facade for a backend agnostic media cache
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.asterisk.org/#/c/487/1/main/media_cache.c
File main/media_cache.c:

Line 216: 		ast_bucket_file_update(bucket_file);
> Can you add some documentation about what this is doing?
This is really asking the bucket backend to check the system it interacts with if the bucket file needs to be updated and - if so - to do so. The bucket file implementation is responsible for actually updating the bucket_file.

That being said, I'm not sure how this could ever work if bucket_file objects are immutable. ast_bucket_file_update does not return a new bucket_file if the update is successful, which means something has to modify the actual bucket_file object. If there is no lock, how can that be done safely?


Line 310: 	bucket_file = ao2_find(media_cache, uri, OBJ_SEARCH_KEY | OBJ_NOLOCK);
> Bucket files are allocated as immutable objects without locks. For both cre
Ouch. That's going to make a fair amount of this ... wrong. I'll update accordingly.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I11227abbf14d8929eeb140ddd101dd5c3820391e
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes