On Thu, 2013-10-31 at 10:02 -0700, Bart Van Assche wrote: > On 31/10/2013 9:21, Or Gerlitz wrote: > > --- 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"); > > }
> 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) ? We want to know about those comparisons, because they often indicate a bug -- either in one's thinking, or in the code they've written. Changing attr.state from unsigned to signed is unlikely to ever happen, as a massive audit would be needed -- this is not an uncommon pattern in the kernel. -- 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
