Author: mjordan Date: Thu Apr 9 07:56:30 2015 New Revision: 434470 URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=434470 Log: clang compiler warnings: Fix autological comparisons
This fixes autological comparison warnings in the following: * chan_skinny: letohl may return a signed or unsigned value, depending on the macro chosen * func_curl: Provide a specific cast to CURLoption to prevent mismatch * cel: Fix enum comparisons where the enum can never be negative * enum: Fix comparison of return result of dn_expand, which returns a signed int value * event: Fix enum comparisons where the enum can never be negative * indications: tone_data.freq1 and freq2 are unsigned, and hence can never be negative * presencestate: Use the actual enum value for INVALID state * security_events: Fix enum comparisons where the enum can never be negative * udptl: Don't bother to check if the return value from encode_length is less than 0, as it returns an unsigned int * translate: Since the parameters are unsigned int, don't bother checking to see if they are negative. The cast to unsigned int would already blow past the matrix bounds. * res_pjsip_exten_state: Use a temporary value to cache the return of ast_hint_presence_state * res_stasis_playback: Fix enum comparisons where the enum can never be negative * res_stasis_recording: Add an enum value for the case where the recording operation is in error; fix enum comparisons * resource_bridges: Use enum value as opposed to -1 * resource_channels: Use enum value as opposed to -1 Review: https://reviewboard.asterisk.org/r/4533 ASTERISK-24917 Reported by: dkdegroot patches: rb4533.patch submitted by dkdegroot (License 6600) ........ Merged revisions 434469 from http://svn.asterisk.org/svn/asterisk/branches/11 Modified: branches/13/ (props changed) branches/13/channels/chan_skinny.c branches/13/channels/pjsip/dialplan_functions.c branches/13/funcs/func_curl.c branches/13/include/asterisk/app.h branches/13/include/asterisk/cel.h branches/13/include/asterisk/logger.h branches/13/include/asterisk/utils.h branches/13/main/app.c branches/13/main/cel.c branches/13/main/enum.c branches/13/main/event.c branches/13/main/indications.c branches/13/main/presencestate.c branches/13/main/security_events.c branches/13/main/udptl.c branches/13/res/ari/resource_bridges.c branches/13/res/ari/resource_channels.c branches/13/res/res_pjsip_exten_state.c branches/13/res/res_stasis_playback.c branches/13/res/res_stasis_recording.c Propchange: branches/13/ ------------------------------------------------------------------------------ Binary property 'branch-11-merged' - no diff available. Modified: branches/13/channels/chan_skinny.c URL: http://svnview.digium.com/svn/asterisk/branches/13/channels/chan_skinny.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/channels/chan_skinny.c (original) +++ branches/13/channels/chan_skinny.c Thu Apr 9 07:56:30 2015 @@ -2374,6 +2374,7 @@ static int transmit_response_bysession(struct skinnysession *s, struct skinny_req *req) { int res = 0; + unsigned long len; if (!s) { ast_log(LOG_WARNING, "Asked to transmit to a non-existent session!\n"); @@ -2382,7 +2383,10 @@ ast_mutex_lock(&s->lock); - if ((letohl(req->len) > SKINNY_MAX_PACKET) || (letohl(req->len) < 0)) { + /* Don't optimize out assigning letohl() to len. It is necessary to guarantee that the comparison will always catch invalid values. + * letohl() may or may not return a signed value depending upon which definition is used. */ + len = letohl(req->len); + if (SKINNY_MAX_PACKET < len) { ast_log(LOG_WARNING, "transmit_response: the length of the request (%u) is out of bounds (%d)\n", letohl(req->len), SKINNY_MAX_PACKET); ast_mutex_unlock(&s->lock); return -1; Modified: branches/13/channels/pjsip/dialplan_functions.c URL: http://svnview.digium.com/svn/asterisk/branches/13/channels/pjsip/dialplan_functions.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/channels/pjsip/dialplan_functions.c (original) +++ branches/13/channels/pjsip/dialplan_functions.c Thu Apr 9 07:56:30 2015 @@ -866,11 +866,11 @@ /* add one since we'll include a comma */ size = strlen(ast_format_get_name(fmt)) + 1; - len -= size; - if ((len) < 0) { + if (len < size) { ao2_ref(fmt, -1); break; } + len -= size; /* no reason to use strncat here since we have already ensured buf has enough space, so strcat can be safely used */ Modified: branches/13/funcs/func_curl.c URL: http://svnview.digium.com/svn/asterisk/branches/13/funcs/func_curl.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/funcs/func_curl.c (original) +++ branches/13/funcs/func_curl.c Thu Apr 9 07:56:30 2015 @@ -171,7 +171,7 @@ #define CURLVERSION_ATLEAST(a,b,c) \ ((LIBCURL_VERSION_MAJOR > (a)) || ((LIBCURL_VERSION_MAJOR == (a)) && (LIBCURL_VERSION_MINOR > (b))) || ((LIBCURL_VERSION_MAJOR == (a)) && (LIBCURL_VERSION_MINOR == (b)) && (LIBCURL_VERSION_PATCH >= (c)))) -#define CURLOPT_SPECIAL_HASHCOMPAT -500 +#define CURLOPT_SPECIAL_HASHCOMPAT ((CURLoption) -500) static void curlds_free(void *data); Modified: branches/13/include/asterisk/app.h URL: http://svnview.digium.com/svn/asterisk/branches/13/include/asterisk/app.h?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/include/asterisk/app.h (original) +++ branches/13/include/asterisk/app.h Thu Apr 9 07:56:30 2015 @@ -985,6 +985,8 @@ * \since 12 */ enum ast_record_if_exists { + /*! Return an Error State for IF_Exists */ + AST_RECORD_IF_EXISTS_ERROR = -1, /*! Fail the recording. */ AST_RECORD_IF_EXISTS_FAIL, /*! Overwrite the existing recording. */ Modified: branches/13/include/asterisk/cel.h URL: http://svnview.digium.com/svn/asterisk/branches/13/include/asterisk/cel.h?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/include/asterisk/cel.h (original) +++ branches/13/include/asterisk/cel.h Thu Apr 9 07:56:30 2015 @@ -39,6 +39,8 @@ * \brief CEL event types */ enum ast_cel_event_type { + AST_CEL_INVALID_VALUE = -1, + AST_CEL_ALL = 0, /*! \brief channel birth */ AST_CEL_CHANNEL_START = 1, /*! \brief channel end */ @@ -75,7 +77,7 @@ AST_CEL_LOCAL_OPTIMIZE = 17, }; -/*! +/*! * \brief Check to see if CEL is enabled * * \since 1.8 Modified: branches/13/include/asterisk/logger.h URL: http://svnview.digium.com/svn/asterisk/branches/13/include/asterisk/logger.h?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/include/asterisk/logger.h (original) +++ branches/13/include/asterisk/logger.h Thu Apr 9 07:56:30 2015 @@ -403,7 +403,7 @@ #define DEBUG_ATLEAST(level) \ (option_debug >= (level) \ - || (ast_opt_dbg_module && ast_debug_get_by_module(AST_MODULE) >= (level))) + || (ast_opt_dbg_module && (int)ast_debug_get_by_module(AST_MODULE) >= (level))) /*! * \brief Log a DEBUG message Modified: branches/13/include/asterisk/utils.h URL: http://svnview.digium.com/svn/asterisk/branches/13/include/asterisk/utils.h?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/include/asterisk/utils.h (original) +++ branches/13/include/asterisk/utils.h Thu Apr 9 07:56:30 2015 @@ -816,7 +816,7 @@ * \param a the array to bound check * \return 0 if value out of bounds, otherwise true (non-zero) */ -#define ARRAY_IN_BOUNDS(v, a) IN_BOUNDS(v, 0, ARRAY_LEN(a) - 1) +#define ARRAY_IN_BOUNDS(v, a) IN_BOUNDS((int) (v), 0, ARRAY_LEN(a) - 1) /* Definition for Digest authorization */ struct ast_http_digest { Modified: branches/13/main/app.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/app.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/main/app.c (original) +++ branches/13/main/app.c Thu Apr 9 07:56:30 2015 @@ -1514,6 +1514,9 @@ break; case AST_RECORD_IF_EXISTS_APPEND: ioflags |= O_APPEND; + break; + case AST_RECORD_IF_EXISTS_ERROR: + ast_assert(0); break; } Modified: branches/13/main/cel.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/cel.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/main/cel.c (original) +++ branches/13/main/cel.c Thu Apr 9 07:56:30 2015 @@ -284,7 +284,7 @@ * \brief Map of ast_cel_event_type to strings */ static const char * const cel_event_types[CEL_MAX_EVENT_IDS] = { - [0] = "ALL", + [AST_CEL_ALL] = "ALL", [AST_CEL_CHANNEL_START] = "CHAN_START", [AST_CEL_CHANNEL_END] = "CHAN_END", [AST_CEL_ANSWER] = "ANSWER", @@ -524,16 +524,13 @@ unsigned int i; for (i = 0; i < ARRAY_LEN(cel_event_types); i++) { - if (!cel_event_types[i]) { - continue; - } - - if (!strcasecmp(name, cel_event_types[i])) { + if (cel_event_types[i] && !strcasecmp(name, cel_event_types[i])) { return i; } } - return -1; + ast_log(LOG_ERROR, "Unknown event name '%s'\n", name); + return AST_CEL_INVALID_VALUE; } static int ast_cel_track_event(enum ast_cel_event_type et) @@ -563,11 +560,10 @@ event_type = ast_cel_str_to_event_type(cur_event); - if (event_type == 0) { + if (event_type == AST_CEL_ALL) { /* All events */ cfg->events = (int64_t) -1; - } else if (event_type == -1) { - ast_log(LOG_ERROR, "Unknown event name '%s'\n", cur_event); + } else if (event_type == AST_CEL_INVALID_VALUE) { return -1; } else { cfg->events |= ((int64_t) 1 << event_type); Modified: branches/13/main/enum.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/enum.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/main/enum.c (original) +++ branches/13/main/enum.c Thu Apr 9 07:56:30 2015 @@ -257,7 +257,7 @@ static int ebl_callback(void *context, unsigned char *answer, int len, unsigned char *fullanswer) { struct ebl_context *c = context; - unsigned int i; + int i; c->pos = 0; /* default to empty */ c->separator[0] = 0; Modified: branches/13/main/event.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/event.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/main/event.c (original) +++ branches/13/main/event.c Thu Apr 9 07:56:30 2015 @@ -199,7 +199,7 @@ type = ast_event_get_type(event); - if (type < 0 || type >= ARRAY_LEN(event_names)) { + if (type >= ARRAY_LEN(event_names)) { ast_log(LOG_ERROR, "Invalid event type - '%u'\n", type); return ""; } Modified: branches/13/main/indications.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/indications.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/main/indications.c (original) +++ branches/13/main/indications.c Thu Apr 9 07:56:30 2015 @@ -359,14 +359,13 @@ if (tone_data.midinote) { /* midi notes must be between 0 and 127 */ - - if (tone_data.freq1 >= 0 && tone_data.freq1 <= 127) { + if (tone_data.freq1 <= 127) { tone_data.freq1 = midi_tohz[tone_data.freq1]; } else { tone_data.freq1 = 0; } - if (tone_data.freq2 >= 0 && tone_data.freq2 <= 127) { + if (tone_data.freq2 <= 127) { tone_data.freq2 = midi_tohz[tone_data.freq2]; } else { tone_data.freq2 = 0; Modified: branches/13/main/presencestate.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/presencestate.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/main/presencestate.c (original) +++ branches/13/main/presencestate.c Thu Apr 9 07:56:30 2015 @@ -290,7 +290,7 @@ state = ast_presence_state_helper(provider, &subtype, &message, 0); - if (state < 0) { + if (state == AST_PRESENCE_INVALID) { return; } Modified: branches/13/main/security_events.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/security_events.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/main/security_events.c (original) +++ branches/13/main/security_events.c Thu Apr 9 07:56:30 2015 @@ -886,7 +886,7 @@ static int check_event_type(const enum ast_security_event_type event_type) { - if (event_type < 0 || event_type >= AST_SECURITY_EVENT_NUM_TYPES) { + if (event_type >= AST_SECURITY_EVENT_NUM_TYPES) { ast_log(LOG_ERROR, "Invalid security event type %u\n", event_type); return -1; } @@ -1172,7 +1172,7 @@ int ast_security_event_report(const struct ast_security_event_common *sec) { - if (sec->event_type < 0 || sec->event_type >= AST_SECURITY_EVENT_NUM_TYPES) { + if (sec->event_type >= AST_SECURITY_EVENT_NUM_TYPES) { ast_log(LOG_ERROR, "Invalid security event type\n"); return -1; } Modified: branches/13/main/udptl.c URL: http://svnview.digium.com/svn/asterisk/branches/13/main/udptl.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/main/udptl.c (original) +++ branches/13/main/udptl.c Thu Apr 9 07:56:30 2015 @@ -362,8 +362,7 @@ } /* Encode the open type */ for (octet_idx = 0; ; num_octets -= enclen, octet_idx += enclen) { - if ((enclen = encode_length(buf, len, num_octets)) < 0) - return -1; + enclen = encode_length(buf, len, num_octets); if (enclen + *len > buflen) { ast_log(LOG_ERROR, "UDPTL (%s): Buffer overflow detected (%u + %u > %u)\n", LOG_TAG(udptl), enclen, *len, buflen); @@ -646,8 +645,7 @@ buf[len++] = 0x00; /* The number of entries will always be zero, so it is pointless allowing for the fragmented case here. */ - if (encode_length(buf, &len, 0) < 0) - return -1; + encode_length(buf, &len, 0); break; case UDPTL_ERROR_CORRECTION_REDUNDANCY: /* Encode the error recovery type */ @@ -658,8 +656,7 @@ entries = s->tx_seq_no; /* The number of entries will always be small, so it is pointless allowing for the fragmented case here. */ - if (encode_length(buf, &len, entries) < 0) - return -1; + encode_length(buf, &len, entries); /* Encode the elements */ for (i = 0; i < entries; i++) { j = (entry - i - 1) & UDPTL_BUF_MASK; Modified: branches/13/res/ari/resource_bridges.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/ari/resource_bridges.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/res/ari/resource_bridges.c (original) +++ branches/13/res/ari/resource_bridges.c Thu Apr 9 07:56:30 2015 @@ -693,7 +693,7 @@ return; } - if (options->if_exists == -1) { + if (options->if_exists == AST_RECORD_IF_EXISTS_ERROR) { ast_ari_response_error( response, 400, "Bad Request", "ifExists invalid"); Modified: branches/13/res/ari/resource_channels.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/ari/resource_channels.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/res/ari/resource_channels.c (original) +++ branches/13/res/ari/resource_channels.c Thu Apr 9 07:56:30 2015 @@ -626,7 +626,7 @@ return; } - if (options->if_exists == -1) { + if (options->if_exists == AST_RECORD_IF_EXISTS_ERROR) { ast_ari_response_error( response, 400, "Bad Request", "ifExists invalid"); Modified: branches/13/res/res_pjsip_exten_state.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_exten_state.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/res/res_pjsip_exten_state.c (original) +++ branches/13/res/res_pjsip_exten_state.c Thu Apr 9 07:56:30 2015 @@ -401,6 +401,7 @@ struct ast_sip_exten_state_data *exten_state_data; char *subtype = NULL; char *message = NULL; + int presence_state; exten_state_data = ao2_alloc(sizeof(*exten_state_data), exten_state_data_destructor); if (!exten_state_data) { @@ -408,11 +409,12 @@ } exten_state_data->exten = exten_state_sub->exten; - if ((exten_state_data->presence_state = ast_hint_presence_state(NULL, exten_state_sub->context, - exten_state_sub->exten, &subtype, &message)) == -1) { + presence_state = ast_hint_presence_state(NULL, exten_state_sub->context, exten_state_sub->exten, &subtype, &message); + if (presence_state == -1 || presence_state == AST_PRESENCE_INVALID) { ao2_cleanup(exten_state_data); return NULL; } + exten_state_data->presence_state = presence_state; exten_state_data->presence_subtype = subtype; exten_state_data->presence_message = message; exten_state_data->user_agent = exten_state_sub->user_agent; Modified: branches/13/res/res_stasis_playback.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_stasis_playback.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/res/res_stasis_playback.c (original) +++ branches/13/res/res_stasis_playback.c Thu Apr 9 07:56:30 2015 @@ -629,9 +629,9 @@ playback_opreation_cb cb; SCOPED_AO2LOCK(lock, playback); - ast_assert(playback->state >= 0 && playback->state < STASIS_PLAYBACK_STATE_MAX); - - if (operation < 0 || operation >= STASIS_PLAYBACK_MEDIA_OP_MAX) { + ast_assert(playback->state < STASIS_PLAYBACK_STATE_MAX); + + if (operation >= STASIS_PLAYBACK_MEDIA_OP_MAX) { ast_log(LOG_ERROR, "Invalid playback operation %u\n", operation); return -1; } Modified: branches/13/res/res_stasis_recording.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_stasis_recording.c?view=diff&rev=434470&r1=434469&r2=434470 ============================================================================== --- branches/13/res/res_stasis_recording.c (original) +++ branches/13/res/res_stasis_recording.c Thu Apr 9 07:56:30 2015 @@ -212,7 +212,7 @@ return AST_RECORD_IF_EXISTS_APPEND; } - return -1; + return AST_RECORD_IF_EXISTS_ERROR; } static void recording_publish(struct stasis_app_recording *recording, const char *cause) @@ -595,13 +595,13 @@ recording_operation_cb cb; SCOPED_AO2LOCK(lock, recording); - if (recording->state < 0 || recording->state >= STASIS_APP_RECORDING_STATE_MAX) { + if (recording->state >= STASIS_APP_RECORDING_STATE_MAX) { ast_log(LOG_WARNING, "Invalid recording state %u\n", recording->state); return -1; } - if (operation < 0 || operation >= STASIS_APP_RECORDING_OPER_MAX) { + if (operation >= STASIS_APP_RECORDING_OPER_MAX) { ast_log(LOG_WARNING, "Invalid recording operation %u\n", operation); return -1; -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits
