[Asterisk-code-review] stringfields: Refactor to allow fields to be added to the e... (asterisk[13])

[George Joseph]

George Joseph has posted comments on this change.

Change subject: stringfields:  Refactor to allow fields to be added to the end of structures
......................................................................


Patch Set 4:

(7 comments)

https://gerrit.asterisk.org/#/c/2477/4/include/asterisk/stringfields.h
File include/asterisk/stringfields.h:

Line 319: #define AST_STRING_FIELD_EXTENDED(name) ast_string_field name
> It really should be made the same and then do casting inside the initializa
Done


https://gerrit.asterisk.org/#/c/2477/4/main/stringfields.c
File main/stringfields.c:

Line 159: 	AST_VECTOR_FREE(&mgr->header->string_fields);
> The original function set all stringfields to the empty string before doing
Yeah, I lost track of what was in the original version.


https://gerrit.asterisk.org/#/c/2477/5/main/stringfields.c
File main/stringfields.c:

Line 115: 	struct ast_string_field_pool *cur = NULL;
> I'm not sure if this is correct.  It might be off by one but I cannot verif
I verified my math. :)


PS5, Line 163: 	 * to free the structure that contains the stringfield manager
             : 	 * and embedded pool anyway, it will be freed as part of that
             : 	 * operation.
             : 	 */
             : 	if ((needed < 0) && mgr->header->embedde
> Something still isn't quite right on when the additional memory pieces need
I think I got it now.  If needed < 1, reset the fields and free everything except the embedded pool if there is one.

If needed == 0, reset the fields and preserve the embedded pool, if there is one, or the last pool, but don't free the internal structure.


PS5, Line 484: 
             : 
> Better names come to mind
Done


Line 487
> Same here
Done


https://gerrit.asterisk.org/#/c/2477/4/tests/test_stringfields.c
File tests/test_stringfields.c:

Line 327: 	inst1 = ast_calloc_with_stringfields(1, struct test_struct, 32);
> Well we could create a C++ delete[] like function that you pass the array a
I can add num_structs, an index and the pool size requested to the new header without breaking ABI.  

ast_calloc_with_stringfields_get_ptr(initial_alloc, 3) would get you the pointer to the structure at index 3.

ast_calloc_with_stringfields_reset(initial_ptr) would use the metadata to reset all the structures.

ast_calloc_with_stringfields_free(initial_ptr) would free any additional pools, vectors, headers and the initial allocation.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I235db338c5b178f5a13b7946afbaa5d4a0f91d61
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes