Skip to content
Snippets Groups Projects
  • George Joseph's avatar
    4d40b161
    stringfields: Refactor to allow fields to be added to the end of structures · 4d40b161
    George Joseph authored
    String fields are great, except that you can't add new ones without breaking
    ABI compatibility because it shifts down everything else in the structure.
    The only alternative is to add your own char * field to the end of the
    structure and manage the memory yourself which isn't ideal, especially since
    you then can't use the OPT_STRINGFIELD_T type.
    
    Background:
    
    The reason string fields had to be declared inside the
    AST_DECLARE_STRING_FIELDS block was to facilitate iteration over all declared
    fields for initialization, compare and copy.  Since AST_DECLARE_STRING_FIELDS
    declared the pool, then the fields, then the manager, you could use the offsets
    of the pool and manager and iterate over the sequential addresses in between to
    access the fields. The actual pool, field allocation and field set operations
    don't actually care where the field is.  It's just iteration over the fields
    that was the problem.
    
    Solution: Extended String Fields
    
    An extended string field is one that is declared outside the
    AST_DECLARE_STRING_FIELDS block but still (anywhere) inside the parent
    structure.  Other than using AST_STRING_FIELD_EXTENDED instead of
    AST_STRING_FIELD, it looks the same as other string fields.  It's storage comes
    from the pool and it participates in string field compare and copy operations
    peformed on the parent structure. It's also a valid target for the
    OPT_STRINGFIELD_T aco option type.
    
    Implementation:
    
    To keep track of the extended fields and make sure that ABI isn't broken, the
    existing embedded_pool pointer in the manager structure was repurposed to be a
    pointer to a separate header structure that contains the embedded_pool pointer
    plus a vector of fields.  The length of the manager structure didn't change and
    the embedded_pool pointer isn't used in the macros, only the stringfields C
    code.  A side benefit of this is that changing the header structure in the
    future won't break ABI.
    
    ast_string_fields_init initializes the normal string fields and appends them to
    the vector, and subsequent calls to ast_string_field_init_extended initialize
    and append the extended fields. Cleanup, ast_string_fields_cmp, and
    ast_string_fields_copy can now work on the vector instead of sequentially
    traversing the addresses between the pool and manager.
    
    The total size of a structure using string fields didn't change, whether using
    extended fields or not, nor have the offsets of any structure members, either
    inside the original block or outside.  Adding an extended field to the end of a
    structure is the same as adding a char *.
    
    Details:
    
    The stringfield C code was pulled out from utils.c and into stringfields.c.
    It just made sense.
    
    Additional work was done in ast_string_field_init and
    ast_calloc_with_stringfields to handle the allocation of the new header
    structure and the vector, and the associated cleanup.  In the process some
    additional NULL pointer checking was added.
    
    A lot of work was done in stringfields.h since the logic for compare and copy
    is there.  Documentation was added as well as somne additional NULL checking.
    
    The ability to call ast_calloc_with_stringfields with a number of structures
    greater than 1 never really worked.  Well, the calloc worked but there was no
    way to access the additional structures or clean them up.  It was agreed that
    there was no use case for requesting more than 1 structure so an ast_assert
    was added to prevent it and the iteration code removed.
    
    Testing:
    
    The stringfield unit tests were updated to test both normal and extended
    fields.  Tests for ast_string_field_ptr_set_by_fields and
    ast_calloc_with_stringfields were also added.
    
    As an ABI test, 13 was compiled from git and the res_pjsip_* modules, except
    res_pjsip itself, saved off.  The patch was then added and a full compile and
    install was performed.  Then the older res_pjsip_* moduled were copied over the
    installed versions so res_pjsip was new and the rest were old.  No issues.
    
    contact->aor, which is a char * at the end of contact, was then changed to an
    extended string field and a recompile and reinstall was performed, again
    leaving stock versions of the the res_pjsip_* modules.  Again, no issues with
    the res_pjsip_* modules using the old stringfield implementation and with
    contact->aor as a char *, and res_pjsip itself using the new stringfield
    implementation and contact->aor being an extended string field.
    
    Finally, several existing string fields were converted to extended string
    fields to test OPT_STRINGFIELD_T.  Again, no issues.
    
    Change-Id: I235db338c5b178f5a13b7946afbaa5d4a0f91d61
    4d40b161
    History
    stringfields: Refactor to allow fields to be added to the end of structures
    George Joseph authored
    String fields are great, except that you can't add new ones without breaking
    ABI compatibility because it shifts down everything else in the structure.
    The only alternative is to add your own char * field to the end of the
    structure and manage the memory yourself which isn't ideal, especially since
    you then can't use the OPT_STRINGFIELD_T type.
    
    Background:
    
    The reason string fields had to be declared inside the
    AST_DECLARE_STRING_FIELDS block was to facilitate iteration over all declared
    fields for initialization, compare and copy.  Since AST_DECLARE_STRING_FIELDS
    declared the pool, then the fields, then the manager, you could use the offsets
    of the pool and manager and iterate over the sequential addresses in between to
    access the fields. The actual pool, field allocation and field set operations
    don't actually care where the field is.  It's just iteration over the fields
    that was the problem.
    
    Solution: Extended String Fields
    
    An extended string field is one that is declared outside the
    AST_DECLARE_STRING_FIELDS block but still (anywhere) inside the parent
    structure.  Other than using AST_STRING_FIELD_EXTENDED instead of
    AST_STRING_FIELD, it looks the same as other string fields.  It's storage comes
    from the pool and it participates in string field compare and copy operations
    peformed on the parent structure. It's also a valid target for the
    OPT_STRINGFIELD_T aco option type.
    
    Implementation:
    
    To keep track of the extended fields and make sure that ABI isn't broken, the
    existing embedded_pool pointer in the manager structure was repurposed to be a
    pointer to a separate header structure that contains the embedded_pool pointer
    plus a vector of fields.  The length of the manager structure didn't change and
    the embedded_pool pointer isn't used in the macros, only the stringfields C
    code.  A side benefit of this is that changing the header structure in the
    future won't break ABI.
    
    ast_string_fields_init initializes the normal string fields and appends them to
    the vector, and subsequent calls to ast_string_field_init_extended initialize
    and append the extended fields. Cleanup, ast_string_fields_cmp, and
    ast_string_fields_copy can now work on the vector instead of sequentially
    traversing the addresses between the pool and manager.
    
    The total size of a structure using string fields didn't change, whether using
    extended fields or not, nor have the offsets of any structure members, either
    inside the original block or outside.  Adding an extended field to the end of a
    structure is the same as adding a char *.
    
    Details:
    
    The stringfield C code was pulled out from utils.c and into stringfields.c.
    It just made sense.
    
    Additional work was done in ast_string_field_init and
    ast_calloc_with_stringfields to handle the allocation of the new header
    structure and the vector, and the associated cleanup.  In the process some
    additional NULL pointer checking was added.
    
    A lot of work was done in stringfields.h since the logic for compare and copy
    is there.  Documentation was added as well as somne additional NULL checking.
    
    The ability to call ast_calloc_with_stringfields with a number of structures
    greater than 1 never really worked.  Well, the calloc worked but there was no
    way to access the additional structures or clean them up.  It was agreed that
    there was no use case for requesting more than 1 structure so an ast_assert
    was added to prevent it and the iteration code removed.
    
    Testing:
    
    The stringfield unit tests were updated to test both normal and extended
    fields.  Tests for ast_string_field_ptr_set_by_fields and
    ast_calloc_with_stringfields were also added.
    
    As an ABI test, 13 was compiled from git and the res_pjsip_* modules, except
    res_pjsip itself, saved off.  The patch was then added and a full compile and
    install was performed.  Then the older res_pjsip_* moduled were copied over the
    installed versions so res_pjsip was new and the rest were old.  No issues.
    
    contact->aor, which is a char * at the end of contact, was then changed to an
    extended string field and a recompile and reinstall was performed, again
    leaving stock versions of the the res_pjsip_* modules.  Again, no issues with
    the res_pjsip_* modules using the old stringfield implementation and with
    contact->aor as a char *, and res_pjsip itself using the new stringfield
    implementation and contact->aor being an extended string field.
    
    Finally, several existing string fields were converted to extended string
    fields to test OPT_STRINGFIELD_T.  Again, no issues.
    
    Change-Id: I235db338c5b178f5a13b7946afbaa5d4a0f91d61