[Asterisk-code-review] vector: Traversal, retrieval, insert and locking enhancements (asterisk[13])

[George Joseph]

George Joseph has posted comments on this change.

Change subject: vector:  Traversal, retrieval, insert and locking enhancements
......................................................................


Patch Set 5:

> > (2 comments)
 > >
 > > If you look at the linked list macros you see that the locking is
 > > handled manually anyway.  The list operations do not
 > automatically
 > > lock/unlock the lock and most of the RWLIST defines are simply
 > > defines to change the name to the normal list define.
 > 
 > That's true. At the same time, one of the main reasons we:
 > * uppercased the vector macros
 > * used macros as opposed to a dedicated struct for the container
 > was to match the linkedlist implementation. It certainly makes it
 > far easier to migrate a linkedlist to a vector (or back) when the
 > implementations are closely aligned.
 > 
 > Given that, I'm not sure why we would be strongly against having
 > macros for the locking. True, it makes that header larger, and it
 > does proliferate the various variants - but it may actually make
 > the code easier to understand, if for no other reason than it is
 > similar to the existing macro implementations.

When I originally started this patch on Friday, my implementation didn't have separate RWVECTOR macros.  I added those to make it look like LIST.  I could revert though.  What I had was an AST_VECTOR_INIT_RWLOCK that initialized the lock, then each of the operation macros just checked for the lock structure and did the appropriate lock/unlock if it found it.  This is more like what container does.  The only penalty for non-locking use was the extra lock pointer in the structure and a check for NULL before each operation.  I actually liked that implementation better but I thought you guys would balk because it didn't look like LIST. :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2e07ecc709d2f5f91bcab8904e5e9340609b00e0
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No