On Fri, Jun 10, 2016 at 05:00:59PM +0300, Leon Romanovsky wrote: > On Thu, Jun 09, 2016 at 04:40:38PM -0600, Jason Gunthorpe wrote: > > On Thu, Jun 09, 2016 at 05:54:07PM -0400, ira.weiny@xxxxxxxxx wrote: > > > > > > +static ssize_t show_fw_ver(struct device *device, struct device_attribute *attr, > > > + char *buf) > > > +{ > > > + char tmp[ETHTOOL_FWVERS_LEN]; > > Very minor request, > Can we avoid variable names like this? > fw_str can be a good candidate for the name. > Sure, but Jason's comment removes the need of this variable. I'll remember for next time. Ira > > > > > + struct ib_device *dev = container_of(device, struct ib_device, dev); > > > + > > > + ib_get_device_fw_str(dev, tmp, sizeof(tmp)); > > > + > > > + return scnprintf(buf, PAGE_SIZE, "%s\n", tmp); > > > > No need to use the stack: > > > > ib_get_device_fw_str(dev, buf, PAGE_SIZE); > > strlcat(buf, "", PAGE_SIZE); > > return strlen(buf); > > > > > &dev_attr_node_guid, > > > - &dev_attr_node_desc > > > + &dev_attr_node_desc, > > > + &dev_attr_fw_ver > > > > Should be > > + &dev_attr_fw_ver, > > > > Note trailing comma. Avoids git churn like the above on append. > > > > Jason > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html