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

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

1 files changed, 241 insertions, 54 deletions

diff --git a/include/asterisk/stringfields.h b/include/asterisk/stringfields.h
index d0879b2ba..c24424b0a 100644
--- a/include/asterisk/stringfields.h
+++ b/include/asterisk/stringfields.h
@@ -17,6 +17,7 @@
  */
 
 /*! \file
+  \page Stringfields String Fields
   \brief String fields in structures
 
   This file contains objects and macros used to manage string
@@ -93,6 +94,80 @@
   ast_free(x);
   \endcode
 
+  A new feature "Extended String Fields" has been added in 13.9.0.
+
+  An extended field is one that is declared outside the AST_DECLARE_STRING_FIELDS
+  block but still inside the parent structure.  It's most useful for extending
+  structures where adding a new string field to an existing AST_DECLARE_STRING_FIELDS
+  block would break ABI compatibility.
+
+  Example:
+
+  \code
+  struct original_structure_version {
+      AST_DECLARE_STRING_FIELDS(
+          AST_STRING_FIELD(foo);
+          AST_STRING_FIELD(bar);
+      );
+      int x1;
+      int x2;
+  };
+  \endcode
+
+  Adding "blah" to the existing string fields breaks ABI compatibility because it changes
+  the offsets of x1 and x2.
+
+  \code
+  struct new_structure_version {
+      AST_DECLARE_STRING_FIELDS(
+          AST_STRING_FIELD(foo);
+          AST_STRING_FIELD(bar);
+          AST_STRING_FIELD(blah);
+      );
+      int x1;
+      int x2;
+  };
+  \endcode
+
+  However, adding "blah" as an extended string field to the end of the structure doesn't break
+  ABI compatibility but still allows the use of the existing pool.
+
+  \code
+  struct new_structure_version {
+      AST_DECLARE_STRING_FIELDS(
+          AST_STRING_FIELD(foo);
+          AST_STRING_FIELD(bar);
+      );
+      int x1;
+      int x2;
+      AST_STRING_FIELD_EXTENDED(blah);
+  };
+  \endcode
+
+  The only additional step required is to call ast_string_field_init_extended so the
+  pool knows about the new field.  It must be called AFTER ast_string_field_init or
+  ast_calloc_with_stringfields.  Although ast_calloc_with_stringfields is used in the
+  sample below, it's not necessary for extended string fields.
+
+  \code
+
+  struct new_structure_version *x = ast_calloc_with_stringfields(1, struct new_structure_version, 252);
+  if (!x) {
+      return;
+  }
+
+  ast_string_field_init_extended(x, blah);
+  \endcode
+
+  The new field can now be treated just like any other string field and it's storage will
+  be released with the rest of the string fields.
+
+  \code
+  ast_string_field_set(x, foo, "infinite loop");
+  ast_stringfield_free_memory(x);
+  ast_free(x);
+  \endcode
+
   This completes the API description.
 */
 
@@ -100,6 +175,7 @@
 #define _ASTERISK_STRINGFIELDS_H
 
 #include "asterisk/inline_api.h"
+#include "asterisk/vector.h"
 
 /*!
   \internal
@@ -139,11 +215,28 @@ struct ast_string_field_pool {
 
 /*!
   \internal
+  \brief The definition for the string field vector used for compare and copy
+  \since 13.9.0
+*/
+AST_VECTOR(ast_string_field_vector, const char **);
+
+/*!
+  \internal
+  \brief Structure used to hold a pointer to the embedded pool and the field vector
+  \since 13.9.0
+*/
+struct ast_string_field_header {
+	struct ast_string_field_pool *embedded_pool;	/*!< pointer to the embedded pool, if any */
+	struct ast_string_field_vector string_fields;	/*!< field vector for compare and copy */
+};
+
+/*!
+  \internal
   \brief Structure used to manage the storage for a set of string fields.
 */
 struct ast_string_field_mgr {
 	ast_string_field last_alloc;			/*!< the last field allocated */
- 	struct ast_string_field_pool *embedded_pool;	/*!< pointer to the embedded pool, if any */
+	struct ast_string_field_header *header;	/*!< pointer to the header */
 #if defined(__AST_DEBUG_MALLOC)
 	const char *owner_file;				/*!< filename of owner */
 	const char *owner_func;				/*!< function name of owner */
@@ -218,6 +311,29 @@ void __ast_string_field_ptr_build_va(struct ast_string_field_mgr *mgr,
 #define AST_STRING_FIELD(name) const ast_string_field name
 
 /*!
+  \brief Declare an extended string field
+  \since 13.9.0
+
+  \param name The field name
+*/
+#define AST_STRING_FIELD_EXTENDED(name) AST_STRING_FIELD(name)
+
+enum ast_stringfield_cleanup_type {
+	/*!
+	 * Reset all string fields and free all extra pools that may have been created
+	 * The allocation or structure can be reused as is.
+	 */
+	AST_STRINGFIELD_RESET = 0,
+	/*!
+	 * Reset all string fields and free all pools.
+	 * If the pointer was returned by ast_calloc_with_stringfields, it can NOT be reused
+	 * and should be immediately freed.  Otherwise, you must call ast_string_field_init
+	 * again if you want to reuse it.
+	 */
+	AST_STRINGFIELD_DESTROY = -1,
+};
+
+/*!
   \brief Declare the fields needed in a structure
   \param field_list The list of fields to declare, using AST_STRING_FIELD() for each one.
   Internally, string fields are stored as a pointer to the head of the pool,
@@ -239,17 +355,65 @@ void __ast_string_field_ptr_build_va(struct ast_string_field_mgr *mgr,
   \brief Initialize a field pool and fields
   \param x Pointer to a structure containing fields
   \param size Amount of storage to allocate.
-	Use 0 to reset fields to the default value,
+	Use AST_STRINGFIELD_RESET to reset fields to the default value,
 	and release all but the most recent pool.
-	size<0 (used internally) means free all pools.
+	AST_STRINGFIELD_DESTROY (used internally) means free all pools which is
+	equivalent to calling ast_string_field_free_memory.
+
   \return 0 on success, non-zero on failure
 */
 #define ast_string_field_init(x, size) \
-	__ast_string_field_init(&(x)->__field_mgr, &(x)->__field_mgr_pool, size, __FILE__, __LINE__, __PRETTY_FUNCTION__)
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL) { \
+		__res__ = __ast_string_field_init(&(x)->__field_mgr, &(x)->__field_mgr_pool, size, __FILE__, __LINE__, __PRETTY_FUNCTION__); \
+	} \
+	__res__ ; \
+})
+
+/*!
+ * \brief free all memory - to be called before destroying the object
+ *
+ * \param x
+ *
+ */
+#define ast_string_field_free_memory(x)	 \
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL) { \
+		__res__ = __ast_string_field_free_memory(&(x)->__field_mgr, &(x)->__field_mgr_pool, \
+			AST_STRINGFIELD_DESTROY, __FILE__, __LINE__, __PRETTY_FUNCTION__); \
+	} \
+	__res__; \
+})
+
+int __ast_string_field_free_memory(struct ast_string_field_mgr *mgr,
+	struct ast_string_field_pool **pool_head, enum ast_stringfield_cleanup_type cleanup_type,
+	const char *file, int lineno, const char *func);
 
-/*! \brief free all memory - to be called before destroying the object */
-#define ast_string_field_free_memory(x)	\
-	__ast_string_field_init(&(x)->__field_mgr, &(x)->__field_mgr_pool, -1, __FILE__, __LINE__, __PRETTY_FUNCTION__)
+/*!
+ * \brief Initialize an extended string field
+ * \since 13.9.0
+ *
+ * \param x Pointer to a structure containing the field
+ * \param field The extended field to initialize
+ * \retval zero on success
+ * \retval non-zero on error
+ *
+ * \note
+ * This macro must be called on ALL fields defined with AST_STRING_FIELD_EXTENDED after
+ * ast_string_field_init has been called.
+ */
+#define ast_string_field_init_extended(x, field) \
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL && (x)->__field_mgr.header != NULL) { \
+		ast_string_field *non_const = (ast_string_field *)&(x)->field; \
+		*non_const = __ast_string_field_empty; \
+		__res__ = AST_VECTOR_APPEND(&(x)->__field_mgr.header->string_fields, non_const); \
+	} \
+	__res__; \
+})
 
 /*!
  * \internal
@@ -260,9 +424,10 @@ int __ast_string_field_init(struct ast_string_field_mgr *mgr, struct ast_string_
 
 /*!
  * \brief Allocate a structure with embedded stringfields in a single allocation
- * \param n Number of structures to allocate (see ast_calloc)
+ * \param n Current imlementation only allows 1 structure to be allocated
  * \param type The type of structure to allocate
  * \param size The number of bytes of space (minimum) to allocate for stringfields to use
+ *             in each structure
  *
  * This function will allocate memory for one or more structures that use stringfields, and
  * also allocate space for the stringfields and initialize the stringfield management
@@ -271,16 +436,16 @@ int __ast_string_field_init(struct ast_string_field_mgr *mgr, struct ast_string_
  * \since 1.8
  */
 #define ast_calloc_with_stringfields(n, type, size) \
-	__ast_calloc_with_stringfields(n, sizeof(type), offsetof(type, __field_mgr), offsetof(type, __field_mgr_pool), \
-				       size, __FILE__, __LINE__, __PRETTY_FUNCTION__)
+	__ast_calloc_with_stringfields(n, sizeof(type), offsetof(type, __field_mgr), \
+		offsetof(type, __field_mgr_pool), size, __FILE__, __LINE__, __PRETTY_FUNCTION__)
 
 /*!
  * \internal
  * \brief internal version of ast_calloc_with_stringfields
  */
-void * attribute_malloc __ast_calloc_with_stringfields(unsigned int num_structs, size_t struct_size, size_t field_mgr_offset,
-						       size_t field_mgr_pool_offset, size_t pool_size, const char *file,
-						       int lineno, const char *func);
+void * attribute_malloc __ast_calloc_with_stringfields(unsigned int num_structs,
+	size_t struct_size, size_t field_mgr_offset, size_t field_mgr_pool_offset, size_t pool_size,
+	const char *file, int lineno, const char *func);
 
 /*!
   \internal
@@ -314,7 +479,14 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
   \retval zero on success
   \retval non-zero on error
 */
-#define ast_string_field_ptr_set(x, ptr, data) ast_string_field_ptr_set_by_fields((x)->__field_mgr_pool, (x)->__field_mgr, ptr, data)
+#define ast_string_field_ptr_set(x, ptr, data) \
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL) { \
+		__res__ = ast_string_field_ptr_set_by_fields((x)->__field_mgr_pool, (x)->__field_mgr, ptr, data); \
+	} \
+	__res__; \
+})
 
 #define ast_string_field_ptr_set_by_fields(field_mgr_pool, field_mgr, ptr, data)               \
 ({                                                                                             \
@@ -348,7 +520,14 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
   \retval zero on success
   \retval non-zero on error
 */
-#define ast_string_field_set(x, field, data) ast_string_field_ptr_set(x, &(x)->field, data)
+#define ast_string_field_set(x, field, data) \
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL) { \
+		__res__ = ast_string_field_ptr_set(x, &(x)->field, data); \
+	} \
+	__res__; \
+})
 
 /*!
   \brief Set a field to a complex (built) value
@@ -359,7 +538,14 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
   \return nothing
 */
 #define ast_string_field_ptr_build(x, ptr, fmt, args...) \
-	__ast_string_field_ptr_build(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) ptr, fmt, args)
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL) { \
+		__ast_string_field_ptr_build(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) ptr, fmt, args); \
+		__res__ = 0; \
+	} \
+	__res__; \
+})
 
 /*!
   \brief Set a field to a complex (built) value
@@ -370,7 +556,14 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
   \return nothing
 */
 #define ast_string_field_build(x, field, fmt, args...) \
-	__ast_string_field_ptr_build(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) &(x)->field, fmt, args)
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL) { \
+		__ast_string_field_ptr_build(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) &(x)->field, fmt, args); \
+		__res__ = 0; \
+	} \
+	__res__; \
+})
 
 /*!
   \brief Set a field to a complex (built) value with prebuilt va_lists.
@@ -381,7 +574,14 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
   \return nothing
 */
 #define ast_string_field_ptr_build_va(x, ptr, fmt, args) \
-	__ast_string_field_ptr_build_va(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) ptr, fmt, args)
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL) { \
+		__ast_string_field_ptr_build_va(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) ptr, fmt, args); \
+		__res__ = 0; \
+	} \
+	__res__; \
+})
 
 /*!
   \brief Set a field to a complex (built) value
@@ -392,7 +592,14 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
   \return nothing
 */
 #define ast_string_field_build_va(x, field, fmt, args) \
-	__ast_string_field_ptr_build_va(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) &(x)->field, fmt, args)
+({ \
+	int __res__ = -1; \
+	if (((void *)(x)) != NULL) { \
+		__ast_string_field_ptr_build_va(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) &(x)->field, fmt, args); \
+		__res__ = 0; \
+	} \
+	__res__; \
+})
 
 /*!
   \brief Compare the string fields in two instances of the same structure
@@ -404,25 +611,16 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
 */
 #define ast_string_fields_cmp(instance1, instance2) \
 ({ \
-	int __res__ = 0; \
-	size_t __ptr_size__ = sizeof(char *); \
-	int __len__ = ((void *)&(instance1)->__field_mgr - (void *)&(instance1)->__field_mgr_pool)/__ptr_size__ - 1; \
-	int __len2__ = ((void *)&(instance2)->__field_mgr - (void *)&(instance2)->__field_mgr_pool)/__ptr_size__ - 1; \
-	if (__len__ == __len2__) { \
-		char **__head1__ = (void *)&(instance1)->__field_mgr_pool + __ptr_size__; \
-		char **__head2__ = (void *)&(instance2)->__field_mgr_pool + __ptr_size__; \
-		for (__len__ -= 1; __len__ >= 0; __len__--) { \
-			__res__ = strcmp(__head1__[__len__], __head2__[__len__]); \
-			if (__res__) { \
-				break; \
-			} \
-		} \
-	} else { \
-		__res__ = -1; \
+	int __res__ = -1; \
+	if (((void *)(instance1)) != NULL && ((void *)(instance2)) != NULL) { \
+		__res__ = __ast_string_fields_cmp(&(instance1)->__field_mgr.header->string_fields, \
+			&(instance2)->__field_mgr.header->string_fields); \
 	} \
 	__res__; \
 })
 
+int __ast_string_fields_cmp(struct ast_string_field_vector *left, struct ast_string_field_vector *right);
+
 /*!
   \brief Copy all string fields from one instance to another of the same structure
   \since 12
@@ -433,27 +631,16 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head,
 */
 #define ast_string_fields_copy(copy, orig) \
 ({ \
-	int __outer_res__ = 0; \
-	size_t __ptr_size__ = sizeof(char *); \
-	int __len__ = ((void *)&(copy)->__field_mgr - (void *)&(copy)->__field_mgr_pool)/__ptr_size__ - 1; \
-	int __len2__ = ((void *)&(orig)->__field_mgr - (void *)&(orig)->__field_mgr_pool)/__ptr_size__ - 1; \
-	if (__len__ == __len2__) { \
-		ast_string_field *__copy_head__ = (void *)&(copy)->__field_mgr_pool + __ptr_size__; \
-		ast_string_field *__orig_head__ = (void *)&(orig)->__field_mgr_pool + __ptr_size__; \
-		for (__len2__ -= 1; __len2__ >= 0; __len2__--) { \
-			__ast_string_field_release_active((copy)->__field_mgr_pool, __copy_head__[__len2__]); \
-			__copy_head__[__len2__] = __ast_string_field_empty; \
-		} \
-		for (__len__ -= 1; __len__ >= 0; __len__--) { \
-			if (ast_string_field_ptr_set((copy), &__copy_head__[__len__], __orig_head__[__len__])) { \
-				__outer_res__ = -1; \
-				break; \
-			} \
-		} \
-	} else { \
-		__outer_res__ = -1; \
+	int __res__ = -1; \
+	if (((void *)(copy)) != NULL && ((void *)(orig)) != NULL) { \
+		__res__ = __ast_string_fields_copy(((copy)->__field_mgr_pool), \
+			(struct ast_string_field_mgr *)&((copy)->__field_mgr), \
+			(struct ast_string_field_mgr *)&((orig)->__field_mgr)); \
 	} \
-	__outer_res__; \
+	__res__; \
 })
 
+int __ast_string_fields_copy(struct ast_string_field_pool *copy_pool,
+	struct ast_string_field_mgr *copy_mgr, struct ast_string_field_mgr *orig_mgr);
+
 #endif /* _ASTERISK_STRINGFIELDS_H */