On 31/10/2013 9:21, Or Gerlitz wrote:
Fix the below few "make W=1" build warnings we have on the IB core.
drivers/infiniband/core/sysfs.c: In function ‘state_show’:
drivers/infiniband/core/sysfs.c:107: warning: comparison of unsigned expression
>= 0 is always true
drivers/infiniband/core/verbs.c: In function ‘ib_modify_qp_is_ok’:
drivers/infiniband/core/verbs.c:783: warning: comparison of unsigned expression
< 0 is always false
drivers/infiniband/core/verbs.c:784: warning: comparison of unsigned expression
< 0 is always false
drivers/infiniband/core/iwcm.c: In function ‘destroy_cm_id’:
drivers/infiniband/core/iwcm.c:330: warning: variable ‘ret’ set but not used
Signed-off-by: Or Gerlitz <[email protected]>
---
drivers/infiniband/core/iwcm.c | 2 --
drivers/infiniband/core/sysfs.c | 2 +-
drivers/infiniband/core/verbs.c | 3 +--
3 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
index c47c203..ab8ebf8 100644
--- a/drivers/infiniband/core/iwcm.c
+++ b/drivers/infiniband/core/iwcm.c
@@ -327,7 +327,6 @@ static void destroy_cm_id(struct iw_cm_id *cm_id)
{
struct iwcm_id_private *cm_id_priv;
unsigned long flags;
- int ret;
cm_id_priv = container_of(cm_id, struct iwcm_id_private, id);
/*
@@ -343,7 +342,6 @@ static void destroy_cm_id(struct iw_cm_id *cm_id)
cm_id_priv->state = IW_CM_STATE_DESTROYING;
spin_unlock_irqrestore(&cm_id_priv->lock, flags);
/* destroy the listening endpoint */
- ret = cm_id->device->iwcm->destroy_listen(cm_id);
spin_lock_irqsave(&cm_id_priv->lock, flags);
break;
case IW_CM_STATE_ESTABLISHED:
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index cde1e7b..1184050 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -104,7 +104,7 @@ static ssize_t state_show(struct ib_port *p, struct
port_attribute *unused,
return ret;
return sprintf(buf, "%d: %s\n", attr.state,
- attr.state >= 0 && attr.state < ARRAY_SIZE(state_name) ?
+ attr.state < ARRAY_SIZE(state_name) ?
state_name[attr.state] : "UNKNOWN");
}
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index a321df2..7b0c1f4 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -780,8 +780,7 @@ int ib_modify_qp_is_ok(enum ib_qp_state cur_state, enum
ib_qp_state next_state,
{
enum ib_qp_attr_mask req_param, opt_param;
- if (cur_state < 0 || cur_state > IB_QPS_ERR ||
- next_state < 0 || next_state > IB_QPS_ERR)
+ if (cur_state > IB_QPS_ERR || next_state > IB_QPS_ERR)
return 0;
if (mask & IB_QP_CUR_STATE &&
I agree that removing unused variables is good because it makes source
code easier to read. However, removing the "attr.state >= 0" check from
state_show() looks dangerous to me. If the type of that variable would
ever be changed from unsigned to signed then that would break
state_show() in a very subtle way. It could take a long time before such
breakage is detected. How about modifying the behavior of W=1 such that
it doesn't warn about comparisons that are always true (-Wno-type-limits) ?
Bart.
--
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
