[Asterisk-code-review] Clang: change previous tautological-compare fixes. (asterisk[master])

[Mark Michelson]

Mark Michelson has submitted this change and it was merged.

Change subject: Clang: change previous tautological-compare fixes.
......................................................................


Clang: change previous tautological-compare fixes.

clang can warn about a so called tautological-compare, when it finds
comparisons which are logically always true, and are therefor deemed
unnecessary.

Exanple:
unsigned int x = 4;
if (x > 0)    // x is always going to be bigger than 0

Enum Case:
Each enumeration is its own type. Enums are an integer type but they
do not have to be *signed*. C leaves it up to the compiler as an
implementation option what to consider the integer type of a particu-
lar enumeration is. Gcc treats an enum without negative values as
an int while clang treats this enum as an unsigned int.

rmudgett & mmichelson: cast the enum to (unsigned int) in assert.
The cast does have an effect. For gcc, which seems to treat all enums
as int, the cast to unsigned int will eliminate the possibility of
negative values being allowed. For clang, which seems to treat enums
without any negative members as unsigned int, the cast will have no
effect. If for some reason in the future a negative value is ever
added to the enum the assert will still catch the negative value.

ASTERISK-24917

Change-Id: I0557ae0154a0b7de68883848a609309cdf0aee6a
---
M main/event.c
M main/security_events.c
M res/res_stasis_playback.c
M res/res_stasis_recording.c
4 files changed, 7 insertions(+), 7 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Richard Mudgett: Looks good to me, but someone else must approve
  Diederik de Groot: Looks good to me, but someone else must approve



diff --git a/main/event.c b/main/event.c
index 228807d..3af0314 100644
--- a/main/event.c
+++ b/main/event.c
@@ -199,7 +199,7 @@
 
 	type = ast_event_get_type(event);
 
-	if (type >= ARRAY_LEN(event_names)) {
+	if ((unsigned int)type >= ARRAY_LEN(event_names)) {
 		ast_log(LOG_ERROR, "Invalid event type - '%u'\n", type);
 		return "";
 	}
diff --git a/main/security_events.c b/main/security_events.c
index 0da62a2..9db932d 100644
--- a/main/security_events.c
+++ b/main/security_events.c
@@ -428,7 +428,7 @@
 	event_type_json = ast_json_object_get(json, "SecurityEvent");
 	event_type = ast_json_integer_get(event_type_json);
 
-	ast_assert(event_type >= 0 && event_type < AST_SECURITY_EVENT_NUM_TYPES);
+	ast_assert((unsigned int)event_type < AST_SECURITY_EVENT_NUM_TYPES);
 
 	if (!(str = ast_str_create(SECURITY_EVENT_BUF_INIT_LEN))) {
 		return NULL;
@@ -886,7 +886,7 @@
 
 static int check_event_type(const enum ast_security_event_type event_type)
 {
-	if (event_type >= AST_SECURITY_EVENT_NUM_TYPES) {
+	if ((unsigned int)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 >= AST_SECURITY_EVENT_NUM_TYPES) {
+	if ((unsigned int)sec->event_type >= AST_SECURITY_EVENT_NUM_TYPES) {
 		ast_log(LOG_ERROR, "Invalid security event type\n");
 		return -1;
 	}
diff --git a/res/res_stasis_playback.c b/res/res_stasis_playback.c
index 2d40ec0..7aac06d 100644
--- a/res/res_stasis_playback.c
+++ b/res/res_stasis_playback.c
@@ -629,7 +629,7 @@
 	playback_opreation_cb cb;
 	SCOPED_AO2LOCK(lock, playback);
 
-	ast_assert(playback->state < STASIS_PLAYBACK_STATE_MAX);
+	ast_assert((unsigned int)playback->state < STASIS_PLAYBACK_STATE_MAX);
 
 	if (operation >= STASIS_PLAYBACK_MEDIA_OP_MAX) {
 		ast_log(LOG_ERROR, "Invalid playback operation %u\n", operation);
diff --git a/res/res_stasis_recording.c b/res/res_stasis_recording.c
index 7af9c10..3a10d41 100644
--- a/res/res_stasis_recording.c
+++ b/res/res_stasis_recording.c
@@ -595,13 +595,13 @@
 	recording_operation_cb cb;
 	SCOPED_AO2LOCK(lock, recording);
 
-	if (recording->state >= STASIS_APP_RECORDING_STATE_MAX) {
+	if ((unsigned int)recording->state >= STASIS_APP_RECORDING_STATE_MAX) {
 		ast_log(LOG_WARNING, "Invalid recording state %u\n",
 			recording->state);
 		return -1;
 	}
 
-	if (operation >= STASIS_APP_RECORDING_OPER_MAX) {
+	if ((unsigned int)operation >= STASIS_APP_RECORDING_OPER_MAX) {
 		ast_log(LOG_WARNING, "Invalid recording operation %u\n",
 			operation);
 		return -1;

-- 
To view, visit https://gerrit.asterisk.org/234
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I0557ae0154a0b7de68883848a609309cdf0aee6a
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Diederik de Groot <dkgroot at talon.nl>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>