diff --git a/include/asterisk/stringfields.h b/include/asterisk/stringfields.h index 51a1696040..4039bf4827 100644 --- a/include/asterisk/stringfields.h +++ b/include/asterisk/stringfields.h @@ -114,6 +114,10 @@ */ typedef const char * ast_string_field; +/* the type of storage used to track how many bytes were allocated for a field */ + +typedef uint16_t ast_string_field_allocation; + /*! \internal \brief A constant empty string used for fields that have no other value @@ -123,13 +127,15 @@ extern const char *__ast_string_field_empty; /*! \internal \brief Structure used to hold a pool of space for string fields + \note base is aligned so base+used can stay aligned by incrementing used with + aligned numbers only */ struct ast_string_field_pool { struct ast_string_field_pool *prev; /*!< pointer to the previous pool, if any */ size_t size; /*!< the total size of the pool */ size_t used; /*!< the space used in the pool */ size_t active; /*!< the amount of space actively in use by fields */ - char base[0]; /*!< storage space for the fields */ + char base[0] __attribute__((aligned(sizeof(ast_string_field_allocation)))); /*!< storage space for the fields */ }; /*! @@ -293,10 +299,6 @@ void * attribute_malloc __ast_calloc_with_stringfields(unsigned int num_structs, void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, const ast_string_field ptr); -/* the type of storage used to track how many bytes were allocated for a field */ - -typedef uint16_t ast_string_field_allocation; - /*! \brief Macro to provide access to the allocation field that lives immediately in front of a string field \param x Pointer to the string field diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h index 44eeb5f8ed..4d015f3ee5 100644 --- a/include/asterisk/utils.h +++ b/include/asterisk/utils.h @@ -740,6 +740,38 @@ static void force_inline _ast_assert(int condition, const char *condition_str, #include "asterisk/strings.h" +/*! + * \brief Add space and let result be a multiple of space. + * \param initial A number to add space to. + * \param space The space to add, this would typically be sizeof(sometype). + * \return The sum of initial plus space plus at most space-1. + * + * Many systems prefer integers to be stored on aligned on memory locations. + * A use case for this is when prepending length fields of type int to a buffer. + * If you keep the total used bytes a multiple of the size of the integer type, + * a next block of length+buffer will have the length field automatically + * aligned. + * + * It looks kind of ugly, but the compiler will optimize this down to 4 or 5 + * inexpensive instructions (on x86_64). + * + * Examples: + * ast_add_and_make_multiple_of(0x18, sizeof(int64_t)) ==> 0x20 + * ast_add_and_make_multiple_of(0x19, sizeof(int64_t)) ==> 0x28 + */ +#define ast_add_and_make_multiple_of(initial, space) (((initial + (2 * space - 1)) / space) * space) + +/*! + * \brief Add bytes so that result is a multiple of size. + * \param initial A number to enlarge. + * \param size The block size the number should be a multiple of. + * \return The sum of initial plus at most size-1. + * + * Similar ast_add_and_make_multiple_of, except that this doesn't add the room + * for the length object, it only ensures that the total is aligned. + */ +#define ast_make_multiple_of(initial, size) (((initial + size - 1) / size) * size) + /*! * \brief An Entity ID is essentially a MAC address, brief and unique */ diff --git a/main/utils.c b/main/utils.c index 8ff6b52154..219d600233 100644 --- a/main/utils.c +++ b/main/utils.c @@ -1652,10 +1652,13 @@ ast_string_field __ast_string_field_alloc_space(struct ast_string_field_mgr *mgr { char *result = NULL; size_t space = (*pool_head)->size - (*pool_head)->used; - size_t to_alloc = needed + sizeof(ast_string_field_allocation); + size_t to_alloc; - /* This +1 accounts for alignment on SPARC */ - if (__builtin_expect(to_alloc + 1 > space, 0)) { + /* Make room for ast_string_field_allocation and make it a multiple of that. */ + to_alloc = ast_add_and_make_multiple_of(needed, sizeof(ast_string_field_allocation)); + ast_assert(to_alloc % sizeof(ast_string_field_allocation) == 0); + + if (__builtin_expect(to_alloc > space, 0)) { size_t new_size = (*pool_head)->size; while (new_size < to_alloc) { @@ -1671,14 +1674,11 @@ ast_string_field __ast_string_field_alloc_space(struct ast_string_field_mgr *mgr #endif } + /* pool->base is always aligned (gcc aligned attribute). We ensure that + * to_alloc is also a multiple of sizeof(ast_string_field_allocation) + * causing result to always be aligned as well; which in turn fixes that + * AST_STRING_FIELD_ALLOCATION(result) is aligned. */ result = (*pool_head)->base + (*pool_head)->used; -#ifdef __sparc__ - /* SPARC requires that the allocation field be aligned. */ - if ((long) result % sizeof(ast_string_field_allocation)) { - result++; - (*pool_head)->used++; - } -#endif (*pool_head)->used += to_alloc; (*pool_head)->active += needed; result += sizeof(ast_string_field_allocation); @@ -1753,13 +1753,10 @@ void __ast_string_field_ptr_build_va(struct ast_string_field_mgr *mgr, available += space; } } else { + /* pool->used is always a multiple of sizeof(ast_string_field_allocation) + * so we don't need to re-align anything here. + */ target = (*pool_head)->base + (*pool_head)->used + sizeof(ast_string_field_allocation); -#ifdef __sparc__ - if ((long) target % sizeof(ast_string_field_allocation)) { - target++; - space--; - } -#endif available = space - sizeof(ast_string_field_allocation); } @@ -1786,15 +1783,15 @@ void __ast_string_field_ptr_build_va(struct ast_string_field_mgr *mgr, __ast_string_field_release_active(*pool_head, *ptr); mgr->last_alloc = *ptr = target; AST_STRING_FIELD_ALLOCATION(target) = needed; - (*pool_head)->used += needed + sizeof(ast_string_field_allocation); + (*pool_head)->used += ast_add_and_make_multiple_of(needed, sizeof(ast_string_field_allocation)); (*pool_head)->active += needed; } else if ((grow = (needed - AST_STRING_FIELD_ALLOCATION(*ptr))) > 0) { /* the allocation was satisfied by using available space in the pool *and* the field was the last allocated field from the pool, so it grew */ - (*pool_head)->used += grow; - (*pool_head)->active += grow; AST_STRING_FIELD_ALLOCATION(*ptr) += grow; + (*pool_head)->used += ast_make_multiple_of(grow, sizeof(ast_string_field_allocation)); + (*pool_head)->active += grow; } }