Re: [PATCH 12/13] IB/core: Export a common fw_ver sysfs entry

["ira.weiny" <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html