[Asterisk-code-review] force inline: remove all force inline from asterisk. (asterisk[13])
Chris Savinovich
asteriskteam at digium.com
Tue Jan 29 18:30:54 CST 2019
Chris Savinovich has uploaded this change for review. ( https://gerrit.asterisk.org/10939
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, 185 insertions(+), 150 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/39/10939/1
diff --git a/include/asterisk/sem.h b/include/asterisk/sem.h
index 6d655d6..436c249 100644
--- a/include/asterisk/sem.h
+++ b/include/asterisk/sem.h
@@ -39,35 +39,12 @@
/* 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);
-}
+int ast_sem_init(struct ast_sem *sem, int pshared, unsigned int value);
+int ast_sem_destroy(struct ast_sem *sem);
+int ast_sem_post(struct ast_sem *sem);
+int ast_sem_wait(struct ast_sem *sem);
+int ast_sem_timedwait(struct ast_sem *sem, const struct timespec *abs_timeout);
+int ast_sem_getvalue(struct ast_sem *sem, int *sval);
#else
/* Unnamed semaphores don't work. Rolling our own, I guess... */
diff --git a/include/asterisk/strings.h b/include/asterisk/strings.h
index 2d66716..94d5314 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.
@@ -1182,10 +1158,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
@@ -1195,16 +1168,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
@@ -1221,16 +1185,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
@@ -1239,16 +1194,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
@@ -1266,19 +1212,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 bcd73aa..d8c076c 100644
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -336,49 +336,11 @@
*/
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
@@ -867,12 +829,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 cb3c4bc..f4e4958 100644
--- a/main/audiohook.c
+++ b/main/audiohook.c
@@ -1424,3 +1424,57 @@
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 7315165..d79a3eb 100644
--- a/main/sem.c
+++ b/main/sem.c
@@ -144,4 +144,36 @@
return 0;
}
+#else
+
+int ast_sem_init(struct ast_sem *sem, int pshared, unsigned int value)
+{
+ return sem_init(&sem->real_sem, pshared, value);
+}
+
+int ast_sem_destroy(struct ast_sem *sem)
+{
+ return sem_destroy(&sem->real_sem);
+}
+
+int ast_sem_post(struct ast_sem *sem)
+{
+ return sem_post(&sem->real_sem);
+}
+
+int ast_sem_wait(struct ast_sem *sem)
+{
+ return sem_wait(&sem->real_sem);
+}
+
+int ast_sem_timedwait(struct ast_sem *sem, const struct timespec *abs_timeout)
+{
+ return sem_timedwait(&sem->real_sem, abs_timeout);
+}
+
+int ast_sem_getvalue(struct ast_sem *sem, int *sval)
+{
+ return sem_getvalue(&sem->real_sem, sval);
+}
+
#endif
diff --git a/main/strings.c b/main/strings.c
index 43fbca4..0100ae7 100644
--- a/main/strings.c
+++ b/main/strings.c
@@ -405,3 +405,84 @@
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 attribute_pure ast_str_hash_restrict(unsigned int hash)
+{
+ return (int) (hash & (unsigned int) INT_MAX);
+}
+
+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 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 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);
+}
+
+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;
+}
+
--
To view, visit https://gerrit.asterisk.org/10939
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2a90249763a7139fd12c54ad4e0e8d5cbbed14f9
Gerrit-Change-Number: 10939
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/81a4528e/attachment-0001.html>
More information about the asterisk-code-review
mailing list