[asterisk-commits] wdoekes: trunk r344846 - in /trunk: ./ include/asterisk/ main/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Nov 11 16:00:20 CST 2011


Author: wdoekes
Date: Fri Nov 11 16:00:14 2011
New Revision: 344846

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=344846
Log:
Use __alignof__ instead of sizeof for stringfield length storage.

Kevin P Fleming suggested that r343157 should use __alignof__ instead
of sizeof. For most systems this won't be an issue, but better fix it
now while it's still fresh.

Review: https://reviewboard.asterisk.org/r/1573
........

Merged revisions 344843 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 344845 from http://svn.asterisk.org/svn/asterisk/branches/10

Modified:
    trunk/   (props changed)
    trunk/include/asterisk/stringfields.h
    trunk/include/asterisk/utils.h
    trunk/main/utils.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.

Modified: trunk/include/asterisk/stringfields.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/stringfields.h?view=diff&rev=344846&r1=344845&r2=344846
==============================================================================
--- trunk/include/asterisk/stringfields.h (original)
+++ trunk/include/asterisk/stringfields.h Fri Nov 11 16:00:14 2011
@@ -135,7 +135,7 @@
 	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] __attribute__((aligned(sizeof(ast_string_field_allocation)))); /*!< storage space for the fields */
+	char base[0] __attribute__((aligned(__alignof__(ast_string_field_allocation)))); /*!< storage space for the fields */
 };
 
 /*!
@@ -302,8 +302,11 @@
 /*!
   \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
-*/
-#define AST_STRING_FIELD_ALLOCATION(x) *((ast_string_field_allocation *) (x - sizeof(ast_string_field_allocation)))
+
+  Note that x must be a pointer to a byte-sized type -- normally (char *) -- or this calculation
+  would break horribly
+*/
+#define AST_STRING_FIELD_ALLOCATION(x) *((ast_string_field_allocation *) (x - __alignof__(ast_string_field_allocation)))
 
 /*!
   \brief Set a field to a simple string value

Modified: trunk/include/asterisk/utils.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/utils.h?view=diff&rev=344846&r1=344845&r2=344846
==============================================================================
--- trunk/include/asterisk/utils.h (original)
+++ trunk/include/asterisk/utils.h Fri Nov 11 16:00:14 2011
@@ -741,36 +741,57 @@
 #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.
+ * \brief Return the number of bytes used in the alignment of type.
+ * \param type
+ * \return The number of bytes required for alignment.
+ *
+ * This is really just __alignof__(), but tucked away in this header so we
+ * don't have to look at the nasty underscores in the source.
+ */
+#define ast_alignof(type) __alignof__(type)
+
+/*!
+ * \brief Increase offset so it is a multiple of the required alignment of type.
+ * \param offset The value that should be increased.
+ * \param type The data type that offset should be aligned to.
+ * \return The smallest multiple of alignof(type) larger than or equal to offset.
+ * \see ast_make_room_for()
  *
  * Many systems prefer integers to be stored on aligned on memory locations.
+ * This macro will increase an offset so a value of the supplied type can be
+ * safely be stored on such a memory location.
+ *
+ * Examples:
+ * ast_align_for(0x17, int64_t) ==> 0x18
+ * ast_align_for(0x18, int64_t) ==> 0x18
+ * ast_align_for(0x19, int64_t) ==> 0x20
+ *
+ * Don't mind the ugliness, the compiler will optimize it.
+ */
+#define ast_align_for(offset, type) (((offset + __alignof__(type) - 1) / __alignof__(type)) * __alignof__(type))
+
+/*!
+ * \brief Increase offset by the required alignment of type and make sure it is
+ *        a multiple of said alignment.
+ * \param offset The value that should be increased.
+ * \param type The data type that room should be reserved for.
+ * \return The smallest multiple of alignof(type) larger than or equal to offset
+ *         plus alignof(type).
+ * \see ast_align_for()
+ *
  * 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,
+ * If you keep the offset a multiple of the alignment 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)
+ * ast_make_room_for(0x17, int64_t) ==> 0x20
+ * ast_make_room_for(0x18, int64_t) ==> 0x20
+ * ast_make_room_for(0x19, int64_t) ==> 0x28
+ *
+ * Don't mind the ugliness, the compiler will optimize it.
+ */
+#define ast_make_room_for(offset, type) (((offset + (2 * __alignof__(type) - 1)) / __alignof__(type)) * __alignof__(type))
 
 /*!
  * \brief An Entity ID is essentially a MAC address, brief and unique 

Modified: trunk/main/utils.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/utils.c?view=diff&rev=344846&r1=344845&r2=344846
==============================================================================
--- trunk/main/utils.c (original)
+++ trunk/main/utils.c Fri Nov 11 16:00:14 2011
@@ -1655,8 +1655,8 @@
 	size_t to_alloc;
 
 	/* 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);
+	to_alloc = ast_make_room_for(needed, ast_string_field_allocation);
+	ast_assert(to_alloc % ast_alignof(ast_string_field_allocation) == 0);
 
 	if (__builtin_expect(to_alloc > space, 0)) {
 		size_t new_size = (*pool_head)->size;
@@ -1675,13 +1675,13 @@
 	}
 
 	/* pool->base is always aligned (gcc aligned attribute). We ensure that
-	 * to_alloc is also a multiple of sizeof(ast_string_field_allocation)
+	 * to_alloc is also a multiple of ast_alignof(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;
 	(*pool_head)->used += to_alloc;
 	(*pool_head)->active += needed;
-	result += sizeof(ast_string_field_allocation);
+	result += ast_alignof(ast_string_field_allocation);
 	AST_STRING_FIELD_ALLOCATION(result) = needed;
 	mgr->last_alloc = result;
 
@@ -1753,11 +1753,11 @@
 			available += space;
 		}
 	} else {
-		/* pool->used is always a multiple of sizeof(ast_string_field_allocation)
+		/* pool->used is always a multiple of ast_alignof(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);
-		available = space - sizeof(ast_string_field_allocation);
+		target = (*pool_head)->base + (*pool_head)->used + ast_alignof(ast_string_field_allocation);
+		available = space - ast_alignof(ast_string_field_allocation);
 	}
 
 	va_copy(ap2, ap);
@@ -1783,14 +1783,14 @@
 		__ast_string_field_release_active(*pool_head, *ptr);
 		mgr->last_alloc = *ptr = target;
 		AST_STRING_FIELD_ALLOCATION(target) = needed;
-		(*pool_head)->used += ast_add_and_make_multiple_of(needed, sizeof(ast_string_field_allocation));
+		(*pool_head)->used += ast_make_room_for(needed, 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
 		*/
 		AST_STRING_FIELD_ALLOCATION(*ptr) += grow;
-		(*pool_head)->used += ast_make_multiple_of(grow, sizeof(ast_string_field_allocation));
+		(*pool_head)->used += ast_align_for(grow, ast_string_field_allocation);
 		(*pool_head)->active += grow;
 	}
 }




More information about the asterisk-commits mailing list