[Asterisk-code-review] force inline: remove all force inline from asterisk. (asterisk[16])
Chris Savinovich
asteriskteam at digium.com
Tue Jan 29 04:09:58 CST 2019
Chris Savinovich has uploaded this change for review. ( https://gerrit.asterisk.org/10935
Change subject: force_inline: remove all force_inline from asterisk.
......................................................................
force_inline: remove all force_inline from asterisk.
A bug in GCC causes arithmetic calculations to fail if the
following conditions are met:
1. TEST_FRAMEWORK on
2. DONT_OPTIMIZE off
3. Fedora and Ubuntu
4. GCC 8.2.1
5. There must exist a certain combination of multithreading.
6. Optimization level -O2 and -O3
7. Flag -fpartial-inline activated (happens automatically when -O2)
Forcing inlining in our code is the actual error trigger. The new
concensus is that it is best to leave it to the compiler to determine
what to inline. Therefore this fix removes all instances of custom
type force_inline wherever possible throughout the Asterisk code.
Change-Id: I2a90249763a7139fd12c54ad4e0e8d5cbbed14f9
---
M include/asterisk/sem.h
M include/asterisk/strings.h
M include/asterisk/utils.h
M main/audiohook.c
M main/sem.c
M main/strings.c
6 files changed, 144 insertions(+), 169 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/35/10935/1
diff --git a/include/asterisk/sem.h b/include/asterisk/sem.h
index fcab82a..c8d254b 100644
--- a/include/asterisk/sem.h
+++ b/include/asterisk/sem.h
@@ -28,50 +28,9 @@
* so see the POSIX documentation for further details.
*/
-#ifdef HAS_WORKING_SEMAPHORE
-/* Working semaphore implementation detected */
#include <semaphore.h>
-struct ast_sem {
- sem_t real_sem;
-};
-
-#define AST_SEM_VALUE_MAX SEM_VALUE_MAX
-
-/* These are thin wrappers; might as well inline them */
-
-static force_inline int ast_sem_init(struct ast_sem *sem, int pshared, unsigned int value)
-{
- return sem_init(&sem->real_sem, pshared, value);
-}
-
-static force_inline int ast_sem_destroy(struct ast_sem *sem)
-{
- return sem_destroy(&sem->real_sem);
-}
-
-static force_inline int ast_sem_post(struct ast_sem *sem)
-{
- return sem_post(&sem->real_sem);
-}
-
-static force_inline int ast_sem_wait(struct ast_sem *sem)
-{
- return sem_wait(&sem->real_sem);
-}
-
-static force_inline int ast_sem_timedwait(struct ast_sem *sem, const struct timespec *abs_timeout)
-{
- return sem_timedwait(&sem->real_sem, abs_timeout);
-}
-
-static force_inline int ast_sem_getvalue(struct ast_sem *sem, int *sval)
-{
- return sem_getvalue(&sem->real_sem, sval);
-}
-
-#else
/* Unnamed semaphores don't work. Rolling our own, I guess... */
#include "asterisk/lock.h"
@@ -173,6 +132,4 @@
*/
int ast_sem_getvalue(struct ast_sem *sem, int *sval);
-#endif
-
#endif /* ASTERISK_SEMAPHORE_H */
diff --git a/include/asterisk/strings.h b/include/asterisk/strings.h
index aaf2737..cf566ef 100644
--- a/include/asterisk/strings.h
+++ b/include/asterisk/strings.h
@@ -91,16 +91,7 @@
\param prefix Prefix to look for.
\param 1 if \a str begins with \a prefix, 0 otherwise.
*/
-static int force_inline attribute_pure ast_begins_with(const char *str, const char *prefix)
-{
- ast_assert(str != NULL);
- ast_assert(prefix != NULL);
- while (*str == *prefix && *prefix != '\0') {
- ++str;
- ++prefix;
- }
- return *prefix == '\0';
-}
+int attribute_pure ast_begins_with(const char *str, const char *prefix);
/*
\brief Checks whether a string ends with another.
@@ -109,22 +100,7 @@
\param suffix Suffix to look for.
\param 1 if \a str ends with \a suffix, 0 otherwise.
*/
-static int force_inline attribute_pure ast_ends_with(const char *str, const char *suffix)
-{
- size_t str_len;
- size_t suffix_len;
-
- ast_assert(str != NULL);
- ast_assert(suffix != NULL);
- str_len = strlen(str);
- suffix_len = strlen(suffix);
-
- if (suffix_len > str_len) {
- return 0;
- }
-
- return strcmp(str + str_len - suffix_len, suffix) == 0;
-}
+int attribute_pure ast_ends_with(const char *str, const char *suffix);
/*!
* \brief return Yes or No depending on the argument.
@@ -1137,10 +1113,7 @@
* (signed) int values. This function restricts an unsigned int hash
* value to the positive half of the (signed) int values.
*/
-static force_inline int attribute_pure ast_str_hash_restrict(unsigned int hash)
-{
- return (int) (hash & (unsigned int) INT_MAX);
-}
+int attribute_pure ast_str_hash_restrict(unsigned int hash);
/*!
* \brief Compute a hash value on a string
@@ -1150,16 +1123,7 @@
*
* http://www.cse.yorku.ca/~oz/hash.html
*/
-static force_inline int attribute_pure ast_str_hash(const char *str)
-{
- unsigned int hash = 5381;
-
- while (*str) {
- hash = hash * 33 ^ (unsigned char) *str++;
- }
-
- return ast_str_hash_restrict(hash);
-}
+int attribute_pure ast_str_hash(const char *str);
/*!
* \brief Compute a hash value on a string
@@ -1176,16 +1140,7 @@
*
* \sa http://www.cse.yorku.ca/~oz/hash.html
*/
-static force_inline int ast_str_hash_add(const char *str, int seed)
-{
- unsigned int hash = (unsigned int) seed;
-
- while (*str) {
- hash = hash * 33 ^ (unsigned char) *str++;
- }
-
- return ast_str_hash_restrict(hash);
-}
+int ast_str_hash_add(const char *str, int seed);
/*!
* \brief Compute a hash value on a case-insensitive string
@@ -1194,16 +1149,7 @@
* all characters to lowercase prior to computing a hash. This
* allows for easy case-insensitive lookups in a hash table.
*/
-static force_inline int attribute_pure ast_str_case_hash(const char *str)
-{
- unsigned int hash = 5381;
-
- while (*str) {
- hash = hash * 33 ^ (unsigned char) tolower(*str++);
- }
-
- return ast_str_hash_restrict(hash);
-}
+int attribute_pure ast_str_case_hash(const char *str);
/*!
* \brief Convert a string to all lower-case
@@ -1221,19 +1167,7 @@
*
* \retval str for convenience
*/
-static force_inline char *attribute_pure ast_str_to_upper(char *str)
-{
- char *str_orig = str;
- if (!str) {
- return str;
- }
-
- for (; *str; ++str) {
- *str = toupper(*str);
- }
-
- return str_orig;
-}
+char *attribute_pure ast_str_to_upper(char *str);
/*!
* \since 12
diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h
index 4dc406d..ccedcfa 100644
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -336,49 +336,10 @@
*/
void ast_unescape_quoted(char *quote_str);
-static force_inline void ast_slinear_saturated_add(short *input, short *value)
-{
- int res;
-
- res = (int) *input + *value;
- if (res > 32767)
- *input = 32767;
- else if (res < -32768)
- *input = -32768;
- else
- *input = (short) res;
-}
-
-static force_inline void ast_slinear_saturated_subtract(short *input, short *value)
-{
- int res;
-
- res = (int) *input - *value;
- if (res > 32767)
- *input = 32767;
- else if (res < -32768)
- *input = -32768;
- else
- *input = (short) res;
-}
-
-static force_inline void ast_slinear_saturated_multiply(short *input, short *value)
-{
- int res;
-
- res = (int) *input * *value;
- if (res > 32767)
- *input = 32767;
- else if (res < -32768)
- *input = -32768;
- else
- *input = (short) res;
-}
-
-static force_inline void ast_slinear_saturated_divide(short *input, short *value)
-{
- *input /= *value;
-}
+void ast_slinear_saturated_add(short *input, short *value);
+void ast_slinear_saturated_subtract(short *input, short *value);
+void ast_slinear_saturated_multiply(short *input, short *value);
+void ast_slinear_saturated_divide(short *input, short *value);
#ifdef localtime_r
#undef localtime_r
@@ -585,12 +546,7 @@
return __VA_ARGS__; \
}\
})
-static void force_inline _ast_assert(int condition, const char *condition_str, const char *file, int line, const char *function)
-{
- if (__builtin_expect(!condition, 1)) {
- __ast_assert_failed(condition, condition_str, file, line, function);
- }
-}
+void _ast_assert(int condition, const char *condition_str, const char *file, int line, const char *function);
#else
#define ast_assert(a)
#define ast_assert_return(a, ...) \
diff --git a/main/audiohook.c b/main/audiohook.c
index 04a379f..819a8c6 100644
--- a/main/audiohook.c
+++ b/main/audiohook.c
@@ -1427,3 +1427,56 @@
return (audiohook ? 0 : -1);
}
+
+void ast_slinear_saturated_add(short *input, short *value)
+{
+ int res;
+
+ res = (int) *input + *value;
+ if (res > 32767)
+ *input = 32767;
+ else if (res < -32768)
+ *input = -32768;
+ else
+ *input = (short) res;
+}
+
+void ast_slinear_saturated_subtract(short *input, short *value)
+{
+ int res;
+
+ res = (int) *input - *value;
+ if (res > 32767)
+ *input = 32767;
+ else if (res < -32768)
+ *input = -32768;
+ else
+ *input = (short) res;
+}
+
+void ast_slinear_saturated_multiply(short *input, short *value)
+{
+ int res;
+
+ res = (int) *input * *value;
+ if (res > 32767)
+ *input = 32767;
+ else if (res < -32768)
+ *input = -32768;
+ else
+ *input = (short) res;
+}
+
+void ast_slinear_saturated_divide(short *input, short *value)
+{
+ *input /= *value;
+}
+
+#ifdef AST_DEVMODE
+void _ast_assert(int condition, const char *condition_str, const char *file, int line, const char *function)
+{
+ if (__builtin_expect(!condition, 1)) {
+ __ast_assert_failed(condition, condition_str, file, line, function);
+ }
+}
+#endif
diff --git a/main/sem.c b/main/sem.c
index cb7b531..b7794f4 100644
--- a/main/sem.c
+++ b/main/sem.c
@@ -26,8 +26,6 @@
#include "asterisk/sem.h"
#include "asterisk/utils.h"
-#ifndef HAS_WORKING_SEMAPHORE
-
/* DIY semaphores! */
int ast_sem_init(struct ast_sem *sem, int pshared, unsigned int value)
@@ -141,5 +139,3 @@
return 0;
}
-
-#endif
diff --git a/main/strings.c b/main/strings.c
index a18bb48..4ac1a79 100644
--- a/main/strings.c
+++ b/main/strings.c
@@ -436,3 +436,82 @@
return str_orig;
}
+int attribute_pure ast_begins_with(const char *str, const char *prefix)
+{
+ ast_assert(str != NULL);
+ ast_assert(prefix != NULL);
+ while (*str == *prefix && *prefix != '\0') {
+ ++str;
+ ++prefix;
+ }
+ return *prefix == '\0';
+}
+
+int attribute_pure ast_ends_with(const char *str, const char *suffix)
+{
+ size_t str_len;
+ size_t suffix_len;
+
+ ast_assert(str != NULL);
+ ast_assert(suffix != NULL);
+ str_len = strlen(str);
+ suffix_len = strlen(suffix);
+
+ if (suffix_len > str_len) {
+ return 0;
+ }
+
+ return strcmp(str + str_len - suffix_len, suffix) == 0;
+}
+
+int ast_str_hash_restrict(unsigned int hash)
+{
+ return (int) (hash & (unsigned int) INT_MAX);
+}
+
+int ast_str_hash(const char *str)
+{
+ unsigned int hash = 5381;
+
+ while (*str) {
+ hash = hash * 33 ^ (unsigned char) *str++;
+ }
+
+ return ast_str_hash_restrict(hash);
+}
+
+int ast_str_hash_add(const char *str, int seed)
+{
+ unsigned int hash = (unsigned int) seed;
+
+ while (*str) {
+ hash = hash * 33 ^ (unsigned char) *str++;
+ }
+
+ return ast_str_hash_restrict(hash);
+}
+
+int ast_str_case_hash(const char *str)
+{
+ unsigned int hash = 5381;
+
+ while (*str) {
+ hash = hash * 33 ^ (unsigned char) tolower(*str++);
+ }
+
+ return ast_str_hash_restrict(hash);
+}
+
+char *ast_str_to_upper(char *str)
+{
+ char *str_orig = str;
+ if (!str) {
+ return str;
+ }
+
+ for (; *str; ++str) {
+ *str = toupper(*str);
+ }
+
+ return str_orig;
+}
--
To view, visit https://gerrit.asterisk.org/10935
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2a90249763a7139fd12c54ad4e0e8d5cbbed14f9
Gerrit-Change-Number: 10935
Gerrit-PatchSet: 1
Gerrit-Owner: Chris Savinovich <csavinovich at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190129/33872065/attachment-0001.html>
More information about the asterisk-code-review
mailing list