opensm: fix some obvious -Wsign-compare warnings

PATCH In review Sasha Khapyorsky 2 participants 4 messages Oct 1, 2009 – Oct 3, 2009 1 message not in the archive InfiniBand

lore

Share & tools

lore

Sasha Khapyorsky

author

2009-10-01 19:25 UTC #0 lore raw

PATCH 1/1 opensm: fix some obvious -Wsign-compare warnings

Fix some obvious (not all) compile warnings generated when -Wsign-compare flag is used.

Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>

---
 opensm/opensm/main.c             |    2 +-
 opensm/opensm/osm_console.c      |    4 ++--
 opensm/opensm/osm_helper.c       |    3 ++-
 opensm/opensm/osm_mcast_mgr.c    |    2 +-
 opensm/opensm/osm_qos_parser_y.y |    2 +-
 opensm/opensm/osm_switch.c       |    4 ++--
 opensm/opensm/osm_ucast_ftree.c  |   10 ++++------
 opensm/opensm/osm_ucast_lash.c   |    2 +-
 8 files changed, 14 insertions(+), 15 deletions(-)

opensm/opensm/main.c +1 -1

index 36a0cc6..2e28c83 100644
@@ -541,7 +541,7 @@ int main(int argc, char *argv[])
 	uint32_t temp, dbg_lvl;
 	boolean_t run_once_flag = FALSE;
 	int32_t vendor_debug = 0;
-	uint32_t next_option;
+	int next_option;
 	char *conf_template = NULL, *config_file = NULL;
 	uint32_t val;
 	const char *const short_option =

opensm/opensm/osm_console.c +2 -2

index d351261..db9a49d 100644
@@ -492,7 +492,7 @@ static void logflush_parse(char **p_last, osm_opensm_t * p_osm, FILE * out)
 
 static void querylid_parse(char **p_last, osm_opensm_t * p_osm, FILE * out)
 {
-	int p = 0;
+	unsigned int p = 0;
 	uint16_t lid = 0;
 	osm_port_t *p_port = NULL;
 	char *p_cmd = next_token(p_last);

@@ -528,7 +528,7 @@ static void querylid_parse(char **p_last, osm_opensm_t * p_osm, FILE * out)
 		p = 1;
 	for ( /* see above */ ; p < p_port->p_node->physp_tbl_size; p++) {
 		fprintf(out,
-			"   Port %d health       : %s\n",
+			"   Port %u health       : %s\n",
 			p,
 			p_port->p_node->physp_table[p].
 			healthy ? "OK" : "ERROR");

opensm/opensm/osm_helper.c +2 -1

index ea9e21f..6b1f55f 100644
@@ -442,7 +442,8 @@ static const char *__ib_sa_attr_str[] = {
 static int sprint_uint8_arr(char *buf, size_t size,
 			    const uint8_t * arr, size_t len)
 {
-	int i, n;
+	int n;
+	unsigned int i;
 	for (i = 0, n = 0; i < len; i++) {
 		n += snprintf(buf + n, size - n, "%s%u", i == 0 ? "" : ",",
 			      arr[i]);

opensm/opensm/osm_mcast_mgr.c +1 -1

index 6f51404..aaab91a 100644
@@ -1004,7 +1004,7 @@ static int mcast_mgr_set_mftables(osm_sm_t * sm)
 	osm_switch_t *p_sw;
 	osm_mcast_tbl_t *p_tbl;
 	int block_notdone, ret = 0;
-	int16_t block_num, max_block = -1;
+	uint16_t block_num, max_block = -1;
 
 	p_sw = (osm_switch_t *) cl_qmap_head(p_sw_tbl);
 	while (p_sw != (osm_switch_t *) cl_qmap_end(p_sw_tbl)) {

opensm/opensm/osm_qos_parser_y.y +1 -1

index 571082b..7a2ce13 100644
@@ -2394,7 +2394,7 @@ static void yyerror(const char *format, ...)
 
 static char * __parser_strip_white(char * str)
 {
-   int i;
+   unsigned int i;
    for (i = (strlen(str)-1); i >= 0; i--)
    {
       if (isspace(str[i]))

opensm/opensm/osm_switch.c +2 -2

index ce1ca63..8a83523 100644
@@ -193,7 +193,7 @@ osm_switch_find_guid_common(IN const osm_switch_t * const p_sw,
 	osm_node_t *p_rem_node;
 	uint64_t sys_guid;
 	uint64_t node_guid;
-	int i;
+	unsigned int i;
 
 	CL_ASSERT(p_sw);

@@ -406,7 +406,7 @@ osm_switch_recommend_path(IN const osm_switch_t * const p_sw,
 			if (routing_for_lmc) {
 				struct osm_remote_guids_count *r = p_port->priv;
 				uint8_t rem_port = osm_physp_get_port_num(p_rem_physp);
-				int j;
+				unsigned int j;
 
 				for (j = 0; j < r->count; j++) {
 					p_remote_guid = &r->guids[j];

opensm/opensm/osm_ucast_ftree.c +4 -6

index 0ec0341..1defd95 100644
@@ -2710,8 +2710,7 @@ static void fabric_route_to_cns(IN ftree_fabric_t * p_ftree)
 	ftree_port_group_t *p_leaf_port_group;
 	ftree_port_group_t *p_hca_port_group;
 	ftree_port_t *p_port;
-	uint32_t i;
-	uint32_t j;
+	unsigned int i, j;
 	uint16_t hca_lid;
 	unsigned routed_targets_on_leaf;

@@ -2788,10 +2787,9 @@ static void fabric_route_to_cns(IN ftree_fabric_t * p_ftree)
 				"Routing %u dummy CAs\n",
 				p_ftree->max_cn_per_leaf -
 				p_sw->down_port_groups_num);
-			for (j = 0;
-			     ((int)j) <
-			     (p_ftree->max_cn_per_leaf -
-			      routed_targets_on_leaf); j++) {
+			for (j = 0; j <
+			     p_ftree->max_cn_per_leaf - routed_targets_on_leaf;
+			     j++) {
 				/* assign downgoing ports by stepping up */
 				fabric_route_downgoing_by_going_up(p_ftree, p_sw,	/* local switch - used as a route-downgoing alg. start point */
 								   NULL,	/* prev. position switch */

opensm/opensm/osm_ucast_lash.c +1 -1

index 63300b5..8b0a190 100644
@@ -70,7 +70,7 @@ static void connect_switches(lash_t * p_lash, int sw1, int sw2, int phy_port_1)
 	mesh_node_t *node = s1->node;
 	switch_t *s2;
 	link_t *l;
-	int i;
+	unsigned int i;
 
 	/*
 	 * if doing mesh analysis:

Signature · 6 lines

1.6.5.rc1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hal Rosenstock

2009-10-02 17:41 UTC #1 lore raw

Sasha,

Quoted 6 lines · Sasha Khapyorsky

Fix some obvious (not all) compile warnings generated when -Wsign-compare flag is used.
Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>
---

<snip...>

opensm/opensm/osm_mcast_mgr.c static int mcast_mgr_set_mftables(osm_sm_t * sm) · line 1007

Quoted 10 lines

opensm/opensm/osm_mcast_mgr.c +1 -1

index 6f51404..aaab91a 100644
@@ -1004,7 +1004,7 @@ static int mcast_mgr_set_mftables(osm_sm_t * sm)
        osm_switch_t *p_sw;
        osm_mcast_tbl_t *p_tbl;
        int block_notdone, ret = 0;
-       int16_t block_num, max_block = -1;
+       uint16_t block_num, max_block = -1;

Doesn't this change break the loop which uses block_num/max_block like:

for (block_num = 0; block_num <= max_block; block_num++) {
...

so should a max_block check be inserted before that loop ?

-- Hal

opensm/opensm/osm_mcast_mgr.c static int mcast_mgr_set_mftables(osm_sm_t * sm) · line 1010

Quoted 2 lines

opensm/opensm/osm_mcast_mgr.c +0 -0

        p_sw = (osm_switch_t *) cl_qmap_head(p_sw_tbl);
        while (p_sw != (osm_switch_t *) cl_qmap_end(p_sw_tbl)) {

<snip...>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Sasha Khapyorsky

author

2009-10-03 00:33 UTC #3 lore raw

In reply to a message not in the archive

Quoted 14 lines · Hal Rosenstock

opensm/opensm/osm_mcast_mgr.c static int mcast_mgr_set_mftables(osm_sm_t * sm) · line 1007
Quoted 11 lines
opensm/opensm/osm_mcast_mgr.c +1 -1
index 6f51404..aaab91a 100644
@@ -1004,7 +1004,7 @@ static int mcast_mgr_set_mftables(osm_sm_t * sm)
        osm_switch_t *p_sw;
        osm_mcast_tbl_t *p_tbl;
        int block_notdone, ret = 0;
-       int16_t block_num, max_block = -1;
+       uint16_t block_num, max_block = -1;
Doesn't this change break the loop which uses block_num/max_block like:

Quoted 2 lines

for (block_num = 0; block_num <= max_block; block_num++) {
...

Yes, it breaks (I was too fast, sorry). The breakage is also catched by Dale. The patch below should fix this.

Sasha

From 0121a81ecb3124760817819cf170bb137523082c Mon Sep 17 00:00:00 2001
From: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>
Date: Sat, 3 Oct 2009 02:19:49 +0200
Subject: [PATCH] opensm: fix endless looping in mcast_mgr

The bug was introduced by broken signed -> unsigned conversion. Fix this properly (including sign-compare warning).

Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>

---
 opensm/include/opensm/osm_switch.h |    2 +-
 opensm/opensm/osm_mcast_mgr.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

opensm/include/opensm/osm_switch.h +1 -1

index e281842..37c1c4f 100644
@@ -103,7 +103,7 @@ typedef struct osm_switch {
 	uint8_t *lft;
 	uint8_t *new_lft;
 	osm_mcast_tbl_t mcast_tbl;
-	uint32_t mft_block_num;
+	int32_t mft_block_num;
 	uint32_t mft_position;
 	unsigned endport_links;
 	unsigned need_update;

opensm/opensm/osm_mcast_mgr.c +1 -1

index aaab91a..6f51404 100644
@@ -1004,7 +1004,7 @@ static int mcast_mgr_set_mftables(osm_sm_t * sm)
 	osm_switch_t *p_sw;
 	osm_mcast_tbl_t *p_tbl;
 	int block_notdone, ret = 0;
-	uint16_t block_num, max_block = -1;
+	int16_t block_num, max_block = -1;
 
 	p_sw = (osm_switch_t *) cl_qmap_head(p_sw_tbl);
 	while (p_sw != (osm_switch_t *) cl_qmap_end(p_sw_tbl)) {

Signature · 6 lines

1.6.5.rc1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sasha Khapyorsky

author

2009-10-03 01:15 UTC #4 lore raw

PATCH 1/1 opensm/osm_qos_parser_y.y: fix endless loop

Fix endless loop introduced by signed --> unsigned conversion (was done for compilation warning fix). Reimplement __parser_strip_white() in slightly shorter variant without explicit index.

Signed-off-by Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/[email protected]>

---
 opensm/opensm/osm_qos_parser_y.y |   25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

opensm/opensm/osm_qos_parser_y.y +11 ~~-14~~

index 7a2ce13..95e420e 100644
@@ -2394,20 +2394,17 @@ static void yyerror(const char *format, ...)
 
 static char * __parser_strip_white(char * str)
 {
-   unsigned int i;
-   for (i = (strlen(str)-1); i >= 0; i--)
-   {
-      if (isspace(str[i]))
-          str[i] = '\0';
-      else
-         break;
-   }
-   for (i = 0; i < strlen(str); i++)
-   {
-      if (!isspace(str[i]))
-         break;
-   }
-   return &(str[i]);
+	char *p;
+
+	while (isspace(*str))
+		str++;
+	if (!*str)
+		return str;
+	p = str + strlen(str) - 1;
+	while (isspace(*p))
+		*p-- = '\0';
+
+	return str;
 }
 
 /***************************************************

Signature · 6 lines

1.6.5.rc1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html