[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)

https://gerrit.asterisk.org/#/c/339/5/include/asterisk/vector.h
File include/asterisk/vector.h:

Line 457: /*!
        :  * \brief Get an address of element in a vector.
        :  *
        :  * \param vec Vector to query.
        :  * \param idx Index of the element to get address of.
        :  */
        : #define AST_VECTOR_GET_ADDR(vec, idx) ({	\
        : 	size_t __idx = (idx);			\
        : 	ast_assert(__idx < (vec)->current);	\
        : 	&(vec)->elems[__idx];			\
        : })
        : 
        : #define AST_RWVECTOR_GET_ADDR(vec, idx) ({ \
        : 	typeof((vec)->elems[0]) res = (typeof((vec)->elems[0]))NULL; \
        : 	if (ast_rwlock_rdlock(&(vec)->lock) == 0) { \
        : 		res = AST_VECTOR_GET_ADDR(vec, idx); \
        : 		ast_rwlock_unlock(&(vec)->lock); \
        : 	} \
        : 	res; \
        : })
> This vector operation is kind of useless for a locked variant.  The returne
The assert could pass but the get could fail if the vector was being modified elsewhere.


Line 521: #define AST_RWVECTOR_GET_CMP(vec, value, cmp) ({ \
> Same for this one.  Unless you only care if the element was in the vector o
The vector could be modified elsewhere while the loop was in progress.


-- 
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: Yes