[Asterisk-code-review] force inline: remove force inline type from Asterisk. (asterisk[16])
Chris Savinovich
asteriskteam at digium.com
Mon Jan 28 23:30:48 CST 2019
Chris Savinovich has uploaded this change for review. ( https://gerrit.asterisk.org/10933
Change subject: force_inline: remove force_inline type from Asterisk.
......................................................................
force_inline: remove force_inline type 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, since it is
best to leave it to the compiler to determine what to inline (or so is
the new consensus). Therefore this fix removes all instances of
type force_inline wherever possible throughout the Asterisk code.
Change-Id: Iad74c085d98eb61132f39d39c977ceb170d21fc9
---
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
M utils/extconf.c
7 files changed, 167 insertions(+), 184 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/33/10933/1
diff --git a/include/asterisk/sem.h b/include/asterisk/sem.h
index fcab82a..306353f 100644
--- a/include/asterisk/sem.h
+++ b/include/asterisk/sem.h
@@ -28,50 +28,10 @@
* 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 +133,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..c4ee51c 100644
--- a/include/asterisk/strings.h
+++ b/include/asterisk/strings.h
@@ -50,22 +50,10 @@
#ifdef AST_DEVMODE
#define ast_strlen_zero(foo) _ast_strlen_zero(foo, __FILE__, __PRETTY_FUNCTION__, __LINE__)
-static force_inline int _ast_strlen_zero(const char *s, const char *file, const char *function, int line)
-{
- if (!s || (*s == '\0')) {
- return 1;
- }
- if (!strcmp(s, "(null)")) {
- ast_log(__LOG_WARNING, file, line, function, "Possible programming error: \"(null)\" is not NULL!\n");
- }
- return 0;
-}
+int _ast_strlen_zero(const char *s, const char *file, const char *function, int line);
#else
-static force_inline int attribute_pure ast_strlen_zero(const char *s)
-{
- return (!s || (*s == '\0'));
-}
+int attribute_pure ast_strlen_zero(const char *s);
#endif
#ifdef SENSE_OF_HUMOR
@@ -91,16 +79,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 +88,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 +1101,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 +1111,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 +1128,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 +1137,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 +1155,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..c8d292c 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..d0124ee 100644
--- a/main/strings.c
+++ b/main/strings.c
@@ -436,3 +436,101 @@
return str_orig;
}
+#ifdef AST_DEVMODE
+int _ast_strlen_zero(const char *s, const char *file, const char *function, int line)
+{
+ if (!s || (*s == '\0')) {
+ return 1;
+ }
+ if (!strcmp(s, "(null)")) {
+ ast_log(__LOG_WARNING, file, line, function, "Possible programming error: \"(null)\" is not NULL!\n");
+ }
+ return 0;
+}
+
+#else
+int attribute_pure ast_strlen_zero(const char *s)
+{
+ return (!s || (*s == '\0'));
+}
+#endif
+
+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;
+}
diff --git a/utils/extconf.c b/utils/extconf.c
index 129dcc8..4e0e1ad 100644
--- a/utils/extconf.c
+++ b/utils/extconf.c
@@ -90,7 +90,7 @@
/* logger.h */
-#define EVENTLOG "event_log"
+#define EVENTLOG "event_log"
#define QUEUELOG "queue_log"
#define DEBUG_M(a) { \
@@ -949,7 +949,7 @@
/* taken from strings.h */
-static force_inline int ast_strlen_zero(const char *s)
+static int ast_strlen_zero(const char *s)
{
return (!s || (*s == '\0'));
}
--
To view, visit https://gerrit.asterisk.org/10933
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: Iad74c085d98eb61132f39d39c977ceb170d21fc9
Gerrit-Change-Number: 10933
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/20190128/8e525d46/attachment-0001.html>
More information about the asterisk-code-review
mailing list