From 89f7196a6e1655ad7c1f875b7559d35ff3a8db90 Mon Sep 17 00:00:00 2001 From: Kathleen King Date: Thu, 3 Jul 2014 10:53:19 -0700 Subject: [PATCH 1/6] Fixed trucation of value warning. There was a parameter mismatch between abs(), which expects an int, and atol() which returns a long. Since max_drift is defined as an int, there is no need to pars q as a long rather than an int. --- src/switch_core_media.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/switch_core_media.c b/src/switch_core_media.c index cd0774eb83..003b123cf5 100644 --- a/src/switch_core_media.c +++ b/src/switch_core_media.c @@ -7654,7 +7654,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_receive_message(switch_core_se maxlen = atol(p); if ((q = strchr(p, ':'))) { q++; - max_drift = abs(atol(q)); + max_drift = abs(atoi(q)); } } } From 24e5c1328372705832b6313559b00ef821cc7749 Mon Sep 17 00:00:00 2001 From: Kathleen King Date: Thu, 3 Jul 2014 12:12:22 -0700 Subject: [PATCH 2/6] Removed an autological-pointer-compare from src/switch_utils.c. Building with Clang 3.5 gave the following warning: error: comparison of array 'iface_out.sin6_addr.__in6_u.__u6_addr8' equal to a null pointer is always false [-Werror,-Wtautological-pointer-compare] This is a problem because as it is written the check will never be true. A pointer to a structure within a structure will never be null. The intention was either to null check the pointer or to check if the IP address itself was not zero. From context in the code this appeared to be a pointer null check so I removed it. --- src/switch_utils.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/switch_utils.c b/src/switch_utils.c index a5c4b8f2f7..da0373098d 100644 --- a/src/switch_utils.c +++ b/src/switch_utils.c @@ -1560,10 +1560,6 @@ SWITCH_DECLARE(switch_status_t) switch_find_local_ip(char *buf, int len, int *ma goto doh; } - if (iface_out.sin6_addr.s6_addr == 0) { - goto doh; - } - inet_ntop(AF_INET6, (const void *) &iface_out.sin6_addr, buf, len - 1); status = SWITCH_STATUS_SUCCESS; } From aef569172b1b115e89dd7b09a40f8c45b22dc4b7 Mon Sep 17 00:00:00 2001 From: Kathleen King Date: Thu, 3 Jul 2014 13:17:12 -0700 Subject: [PATCH 3/6] Removed a useless called to abs. Clang 3.5 reported the following error: error: taking the absolute value of unsigned type 'unsigned int' has no effect [-Werror,-Wabsolute-value] Subtracting unsigned variables will never be negative and will either be the small expected value or will wrap to a very big value. This code is trying to determine if the difference between these timestamps is greater than 16000. The variables last_write_ts and this_ts deal with timestamps. In the normal case this_ts will be a larger timestamp than last_write_ts. This change will maintain the intended behavior of reseting the video if the difference is larger than 16000 and in the abnormal case this value would wrap and still exceed the 16000. --- src/switch_rtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/switch_rtp.c b/src/switch_rtp.c index eccce9d71c..de6a238852 100644 --- a/src/switch_rtp.c +++ b/src/switch_rtp.c @@ -6341,7 +6341,7 @@ static int rtp_common_write(switch_rtp_t *rtp_session, if (!switch_rtp_test_flag(rtp_session, SWITCH_RTP_FLAG_VIDEO)) { this_ts = ntohl(send_msg->header.ts); - if (abs(rtp_session->last_write_ts - this_ts) > 16000) { + if ((this_ts - rtp_session->last_write_ts) > 16000) { rtp_session->flags[SWITCH_RTP_FLAG_RESET] = 1; } From 85699480b3318978f724d6aec3fe00b742a6db78 Mon Sep 17 00:00:00 2001 From: Kathleen King Date: Thu, 3 Jul 2014 14:41:24 -0700 Subject: [PATCH 4/6] Fixed trucation of value warning. There was a parameter mismatch between abs(), which expects an int, and atol() which returns a long. Since max_drift is defined as an int, there is no need to pars q as a long rather than an int. --- src/mod/endpoints/mod_sofia/rtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mod/endpoints/mod_sofia/rtp.c b/src/mod/endpoints/mod_sofia/rtp.c index 3f704c5f10..897534970e 100644 --- a/src/mod/endpoints/mod_sofia/rtp.c +++ b/src/mod/endpoints/mod_sofia/rtp.c @@ -622,7 +622,7 @@ static switch_status_t channel_receive_message(switch_core_session_t *session, s maxlen = atol(p); if ((q = strchr(p, ':'))) { q++; - max_drift = abs(atol(q)); + max_drift = abs(atoi(q)); } } } From 2d85726ecd5bfb42de0563cdead31eaed21a303b Mon Sep 17 00:00:00 2001 From: Kathleen King Date: Thu, 3 Jul 2014 17:03:01 -0700 Subject: [PATCH 5/6] Fixed dead code. While reviewing code I noticed some dead code. It was not possible to default to the channel variable because the parameter could not be both full and empty. If the parameter is not a non zero length string then the code looked like it was intending to default to the channel variable 'presence_data_cols'. If neither of these are the case it is a noop. By enabling the dead code, you now have access to set the 'presence_data_cols' in the dialplan or scripts like lua. --- src/switch_channel.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/switch_channel.c b/src/switch_channel.c index 2168fd396e..c27f1c5ff9 100644 --- a/src/switch_channel.c +++ b/src/switch_channel.c @@ -1336,32 +1336,30 @@ SWITCH_DECLARE(switch_status_t) switch_channel_transfer_variable_prefix(switch_c SWITCH_DECLARE(void) switch_channel_set_presence_data_vals(switch_channel_t *channel, const char *presence_data_cols) { - if (!zstr(presence_data_cols)) { - char *cols[128] = { 0 }; - char header_name[128] = ""; - int col_count = 0, i = 0; - char *data_copy = NULL; - + char *cols[128] = { 0 }; + char header_name[128] = ""; + int col_count = 0, i = 0; + char *data_copy = NULL; + + if (zstr(presence_data_cols)) { + presence_data_cols = switch_channel_get_variable_dup(channel, "presence_data_cols", SWITCH_FALSE, -1); if (zstr(presence_data_cols)) { - presence_data_cols = switch_channel_get_variable_dup(channel, "presence_data_cols", SWITCH_FALSE, -1); - if (zstr(presence_data_cols)) { - return; - } + return; } - - data_copy = strdup(presence_data_cols); - - col_count = switch_split(data_copy, ':', cols); - - for (i = 0; i < col_count; i++) { - const char *val = NULL; - switch_snprintf(header_name, sizeof(header_name), "PD-%s", cols[i]); - val = switch_channel_get_variable(channel, cols[i]); - switch_channel_set_profile_var(channel, header_name, val); - } - - switch_safe_free(data_copy); } + + data_copy = strdup(presence_data_cols); + + col_count = switch_split(data_copy, ':', cols); + + for (i = 0; i < col_count; i++) { + const char *val = NULL; + switch_snprintf(header_name, sizeof(header_name), "PD-%s", cols[i]); + val = switch_channel_get_variable(channel, cols[i]); + switch_channel_set_profile_var(channel, header_name, val); + } + + switch_safe_free(data_copy); } From 1affff9db4aaa29ab66f9f5db76f575eeabd86b5 Mon Sep 17 00:00:00 2001 From: Kathleen King Date: Thu, 3 Jul 2014 18:51:14 -0700 Subject: [PATCH 6/6] Fixed a clang-3.5 missing-prototype warning and added doxygen documentation for switch_channel_set_presence_data_vals. #doxygen --- src/include/switch_channel.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/include/switch_channel.h b/src/include/switch_channel.h index aea8c217d2..a305c96a45 100644 --- a/src/include/switch_channel.h +++ b/src/include/switch_channel.h @@ -165,6 +165,13 @@ SWITCH_DECLARE(switch_status_t) switch_channel_alloc(_In_ switch_channel_t **cha SWITCH_DECLARE(switch_status_t) switch_channel_init(switch_channel_t *channel, switch_core_session_t *session, switch_channel_state_t state, switch_channel_flag_t flag); +/*! + \brief Takes presence_data_cols as a parameter or as a channel variable and copies them to channel profile variables + \param channel the channel on which to set the channel profile variables + \param presence_data_cols is a colon separated list of channel variables to copy to channel profile variables + */ +SWITCH_DECLARE(void) switch_channel_set_presence_data_vals(switch_channel_t *channel, const char *presence_data_cols); + /*! \brief Fire A presence event for the channel \param channel the channel to initilize