[Asterisk-code-review] vector: Add REMOVE, INSERT SORTED and RESET macros (asterisk[13])

[Corey Farrell]

Corey Farrell has posted comments on this change.

Change subject: vector:  Add REMOVE, INSERT_SORTED and RESET macros
......................................................................


Patch Set 5: Code-Review-1

(3 comments)

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

Line 276:  * \brief Insert an element into a sorted vector
I'm still unsure it's appropriate for this macro to be called 'insert'.  The difference is in how it handles two objects with the same sort value.  It looks to me like it will do an 'add', so a second object with equal sort value will be added after all existing items with equal sort.


Line 473: 	size_t idx; \
        : 	for (idx = 0; idx < (vec)->current; ++idx) { \
        : 		cleanup((vec)->elems[idx]); \
        : 	} \
This pasted code should be replaced with a call to AST_VECTOR_CALLBACK_VOID then?


https://gerrit.asterisk.org/#/c/421/5/tests/test_vector.c
File tests/test_vector.c:

Line 351: 	ast_test_validate_cleanup(test, AST_VECTOR_GET(&sv1, 2) == CCC, rc, cleanup);
        : 	ast_test_validate_cleanup(test, AST_VECTOR_GET(&sv1, 3) == CCC, rc, cleanup);
This isn't effective at verifying the order of elements with the same sort result.  To do that I think you need:
AST_VECTOR(test_struct, int *) sv1;
int CCC = 5, CCCdup = 5;

AST_VECTOR_INSERT_SORTED(&sv1, &CCC);
AST_VECTOR_INSERT_SORTED(&sv1, &CCCdup);

This way you can verify that &CCC and &CCCdup are in the list in the correct order.  This is more in line with the normal use case where the sort is done on a field of a structure, and the struct pointer is added to the vector.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41d32dbdf7137e0557134efeff9f9f1064b58d14
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: Y Ateya <y.ateya at starkbits.com>
Gerrit-HasComments: Yes